Merge pull request #138 from IaroslavMazur/iaro/test-changes-after-audit

Changes in test/ resulting from the audit
sablier-labs · Jul 10, 2023 · 72f3a9d · 72f3a9d
2 parents 5b37f74 + bb739fd
commit 72f3a9d
Show file tree

Hide file tree

Showing 14 changed files with 40 additions and 27 deletions.
diff --git a/test/Base.t.sol b/test/Base.t.sol
@@ -224,7 +224,7 @@ abstract contract Base_Test is Assertions, Events, StdCheats, V2CoreUtils {
         vm.expectCall({ callee: asset_, data: abi.encodeCall(IERC20.transferFrom, (from, to, amount)) });
     }
 
-    /// @dev Expects multiple similar calls to {ISablierV2LockupDynamic.createWithDeltas}, each with the specified
+    /// @dev Expects multiple calls to {ISablierV2LockupDynamic.createWithDeltas}, each with the specified
     /// `params`.
     function expectMultipleCallsToCreateWithDeltas(
         uint64 count,
@@ -239,7 +239,7 @@ abstract contract Base_Test is Assertions, Events, StdCheats, V2CoreUtils {
         });
     }
 
-    /// @dev Expects multiple similar calls to {ISablierV2LockupDynamic.createWithDurations}, each with the specified
+    /// @dev Expects multiple calls to {ISablierV2LockupDynamic.createWithDurations}, each with the specified
     /// `params`.
     function expectMultipleCallsToCreateWithDurations(
         uint64 count,
@@ -254,7 +254,7 @@ abstract contract Base_Test is Assertions, Events, StdCheats, V2CoreUtils {
         });
     }
 
-    /// @dev Expects multiple similar calls to {ISablierV2LockupDynamic.createWithMilestones}, each with the specified
+    /// @dev Expects multiple calls to {ISablierV2LockupDynamic.createWithMilestones}, each with the specified
     /// `params`.
     function expectMultipleCallsToCreateWithMilestones(
         uint64 count,
@@ -269,7 +269,7 @@ abstract contract Base_Test is Assertions, Events, StdCheats, V2CoreUtils {
         });
     }
 
-    /// @dev Expects multiple similar calls to {ISablierV2LockupDynamic.createWithRange}, each with the specified
+    /// @dev Expects multiple calls to {ISablierV2LockupDynamic.createWithRange}, each with the specified
     /// `params`.
     function expectMultipleCallsToCreateWithRange(uint64 count, LockupLinear.CreateWithRange memory params) internal {
         vm.expectCall({

diff --git a/test/fork/plugin/onStreamCanceled.t.sol b/test/fork/plugin/onStreamCanceled.t.sol
@@ -9,7 +9,7 @@ import { Fork_Test } from "../Fork.t.sol";
 
 import { Permit2Params } from "src/types/Permit2.sol";
 
-/// @dev Runs against multiple assets.
+/// @dev Runs against multiple fork assets.
 abstract contract OnStreamCanceled_Fork_Test is Fork_Test, PermitSignature {
     constructor(IERC20 asset_) Fork_Test(asset_) { }
 

diff --git a/test/fork/target/batchCancelMultiple.t.sol b/test/fork/target/batchCancelMultiple.t.sol
@@ -8,7 +8,7 @@ import { Batch } from "src/types/DataTypes.sol";
 
 import { Fork_Test } from "../Fork.t.sol";
 
-/// @dev Runs against multiple assets.
+/// @dev Runs against multiple fork assets.
 abstract contract BatchCancelMultiple_Fork_Test is Fork_Test {
     constructor(IERC20 asset_) Fork_Test(asset_) { }
 

diff --git a/test/fork/target/batchCreate.t.sol b/test/fork/target/batchCreate.t.sol
@@ -14,7 +14,7 @@ import { Fork_Test } from "../Fork.t.sol";
 import { ArrayBuilder } from "../../utils/ArrayBuilder.sol";
 import { BatchBuilder } from "../../utils/BatchBuilder.sol";
 
-/// @dev Runs against multiple assets.
+/// @dev Runs against multiple fork assets.
 abstract contract BatchCreate_Fork_Test is Fork_Test, PermitSignature {
     constructor(IERC20 asset_) Fork_Test(asset_) { }
 

diff --git a/test/integration/archive/list/list.t.sol b/test/integration/archive/list/list.t.sol
@@ -33,4 +33,10 @@ contract List_Integration_Test is Integration_Test {
         bool isListed = archive.isListed(address(lockupLinear));
         assertTrue(isListed, "isListed");
     }
+
+    function test_List_Event() external callerAdmin addressNotListed {
+        vm.expectEmit();
+        emit List({ admin: users.admin.addr, addr: address(lockupLinear) });
+        archive.list(address(lockupLinear));
+    }
 }
diff --git a/test/integration/archive/unlist/unlist.t.sol b/test/integration/archive/unlist/unlist.t.sol
@@ -18,18 +18,25 @@ contract Unlist_Integration_Test is Integration_Test {
     }
 
     function test_Unlist_AddressNotListed() external callerAdmin {
+        archive.unlist(address(lockupLinear));
         bool isListed = archive.isListed(address(lockupLinear));
         assertFalse(isListed, "isListed");
     }
 
     modifier addressListed() {
+        archive.list(address(lockupLinear));
         _;
     }
 
     function test_Unlist() external callerAdmin addressListed {
-        archive.list(address(lockupLinear));
         archive.unlist(address(lockupLinear));
         bool isListed = archive.isListed(address(lockupLinear));
         assertFalse(isListed, "isListed");
     }
+
+    function test_Unlist_Event() external callerAdmin addressListed {
+        vm.expectEmit({ emitter: address(archive) });
+        emit Unlist({ admin: users.admin.addr, addr: address(lockupLinear) });
+        archive.unlist(address(lockupLinear));
+    }
 }
diff --git a/test/integration/plugin/on-stream-canceled/onStreamCanceled.tree b/test/integration/plugin/on-stream-canceled/onStreamCanceled.tree
@@ -2,7 +2,7 @@ onStreamCanceled.t.sol
 ├── when not delegate called
 │  └── it should revert
 └── when delegate called
-   ├── when the caller is not listed
+   ├── when the caller is not listed in the archive
    │  └── it should revert
-   └── when the caller is listed
+   └── when the caller is listed in the archive
       └── it should forward the assets to the proxy owner
diff --git a/test/integration/target/batch-cancel-multiple/batchCancelMultiple.t.sol b/test/integration/target/batch-cancel-multiple/batchCancelMultiple.t.sol
@@ -33,7 +33,7 @@ contract BatchCancelMultiple_Integration_Test is Integration_Test {
     }
 
     function test_BatchCancelMultiple() external batchSizeNotZero whenDelegateCalled {
-        // Create two batches of streams due to be canceled.
+        // Create two batches of streams to be canceled.
         uint256[] memory dynamicStreamIds = batchCreateWithMilestones();
         uint256[] memory linearStreamIds = batchCreateWithRange();
 

diff --git a/test/integration/target/batch-create-with-range/batchCreateWithRange.t.sol b/test/integration/target/batch-create-with-range/batchCreateWithRange.t.sol
@@ -9,10 +9,10 @@ import { Integration_Test } from "../../Integration.t.sol";
 
 contract BatchCreateWithRange_Integration_Test is Integration_Test {
     function test_RevertWhen_NotDelegateCalled() external {
-        Batch.CreateWithDurations[] memory batch;
+        Batch.CreateWithRange[] memory batch;
         Permit2Params memory permit2Params;
         vm.expectRevert(Errors.CallNotDelegateCall.selector);
-        target.batchCreateWithDurations(lockupLinear, asset, batch, permit2Params);
+        target.batchCreateWithRange(lockupLinear, asset, batch, permit2Params);
     }
 
     modifier whenDelegateCalled() {

diff --git a/test/integration/target/burn/burn.t.sol b/test/integration/target/burn/burn.t.sol
@@ -44,7 +44,7 @@ contract Burn_Integration_Test is Integration_Test {
         data = abi.encodeCall(target.burn, (lockup, streamId));
         aliceProxy.execute(address(target), data);
 
-        // Expect the NFT owner not to exist anymore.
+        // Expect the NFT owner to not exist anymore.
         vm.expectRevert("ERC721: invalid token ID");
         lockup.getRecipient(streamId);
     }

diff --git a/test/integration/target/cancel-and-create/cancelAndCreate.t.sol b/test/integration/target/cancel-and-create/cancelAndCreate.t.sol
@@ -61,7 +61,7 @@ contract CancelAndCreate_Integration_Test is Integration_Test {
         assertEq(actualNewStreamId, expectedNewStreamId, "new stream id mismatch");
     }
 
-    function test_CancelAndCreateWithDeltas_AcrossSablierContracts() external {
+    function test_CancelAndCreateWithDeltas_AcrossSablierContracts() external whenDelegateCalled {
         // Create the stream due to be canceled.
         uint256 streamId = createWithRange();
 
@@ -130,7 +130,7 @@ contract CancelAndCreate_Integration_Test is Integration_Test {
         assertEq(actualNewStreamId, expectedNewStreamId, "new stream id mismatch");
     }
 
-    function test_CancelAndCreateWithDurations_AcrossSablierContracts() external {
+    function test_CancelAndCreateWithDurations_AcrossSablierContracts() external whenDelegateCalled {
         // Create the stream due to be canceled.
         uint256 streamId = createWithMilestones();
 
@@ -199,7 +199,7 @@ contract CancelAndCreate_Integration_Test is Integration_Test {
         assertEq(actualNewStreamId, expectedNewStreamId, "new stream id mismatch");
     }
 
-    function test_CancelAndCreateWithMilestones_AcrossSablierContracts() external {
+    function test_CancelAndCreateWithMilestones_AcrossSablierContracts() external whenDelegateCalled {
         // Create the stream due to be canceled.
         uint256 streamId = createWithRange();
 
@@ -268,7 +268,7 @@ contract CancelAndCreate_Integration_Test is Integration_Test {
         assertEq(actualNewStreamId, expectedNewStreamId, "new stream id mismatch");
     }
 
-    function test_CancelAndCreateWithRange_AcrossSablierContracts() external {
+    function test_CancelAndCreateWithRange_AcrossSablierContracts() external whenDelegateCalled {
         // Create the stream due to be canceled.
         uint256 streamId = createWithMilestones();
 

diff --git a/test/integration/target/cancel/cancel.t.sol b/test/integration/target/cancel/cancel.t.sol
@@ -32,7 +32,7 @@ contract Cancel_Integration_Test is Integration_Test {
         // Simulate the passage of time.
         vm.warp(defaults.CLIFF_TIME());
 
-        // Asset flow: proxy owner → proxy → proxy owner
+        // Asset flow: Sablier → proxy → proxy owner
         expectCallToTransfer({ to: address(aliceProxy), amount: defaults.REFUND_AMOUNT() });
         expectCallToTransfer({ to: users.alice.addr, amount: defaults.REFUND_AMOUNT() });
 

diff --git a/test/integration/target/wrap-and-create/wrapAndCreate.t.sol b/test/integration/target/wrap-and-create/wrapAndCreate.t.sol
@@ -9,10 +9,10 @@ import { Errors } from "src/libraries/Errors.sol";
 import { Integration_Test } from "../../Integration.t.sol";
 
 /// @dev This contracts tests the following functions:
-/// - `wrapEtherAndCreateWithDeltas`
-/// - `wrapEtherAndCreateWithDurations`
-/// - `wrapEtherAndCreateWithMilestones`
-/// - `wrapEtherAndCreateWithRange`
+/// - `wrapAndCreateWithDeltas`
+/// - `wrapAndCreateWithDurations`
+/// - `wrapAndCreateWithMilestones`
+/// - `wrapAndCreateWithRange`
 contract WrapAndCreate_Integration_Test is Integration_Test {
     modifier whenDelegateCalled() {
         _;

diff --git a/test/utils/BatchBuilder.sol b/test/utils/BatchBuilder.sol
@@ -6,7 +6,7 @@ import { LockupDynamic, LockupLinear } from "@sablier/v2-core/types/DataTypes.so
 import { Batch } from "../../src/types/DataTypes.sol";
 
 library BatchBuilder {
-    /// @notice Generates an array of `batchSingle` with the specified `batchSize`.
+    /// @notice Generates an array containing `batchSize` copies of `batchSingle`.
     function fillBatch(
         Batch.CreateWithDeltas memory batchSingle,
         uint256 batchSize
@@ -44,7 +44,7 @@ library BatchBuilder {
         batch = fillBatch(batchSingle, batchSize);
     }
 
-    /// @notice Generates an array of `batchSingle` with the specified `batchSize`.
+    /// @notice Generates an array containing `batchSize` copies of `batchSingle`.
     function fillBatch(
         Batch.CreateWithDurations memory batchSingle,
         uint256 batchSize
@@ -82,7 +82,7 @@ library BatchBuilder {
         batch = fillBatch(batchSingle, batchSize);
     }
 
-    /// @notice Generates an array of `batchSingle` with the specified `batchSize`.
+    /// @notice Generates an array containing `batchSize` copies of `batchSingle`.
     function fillBatch(
         Batch.CreateWithMilestones memory batchSingle,
         uint256 batchSize
@@ -121,7 +121,7 @@ library BatchBuilder {
         batch = fillBatch(batchSingle, batchSize);
     }
 
-    /// @notice Generates an array of `batchSingle` with the specified `batchSize`.
+    /// @notice Generates an array containing `batchSize` copies of `batchSingle`.
     function fillBatch(
         Batch.CreateWithRange memory batchSingle,
         uint256 batchSize