Merge pull request #7 from metadock/tests/cancel-invoice

Implement `cancelInvoice` integration tests

src/modules/invoice-module/InvoiceModule.sol

@@ -213,27 +213,33 @@ contract InvoiceModule is IInvoiceModule, StreamManager {
  if (invoice.status == Types.Status.Paid) {
  revert Errors.CannotCancelPaidInvoice();
  } else if (invoice.status == Types.Status.Canceled) {
- revert Errors.CannotCancelCanceledInvoice();
+ revert Errors.InvoiceAlreadyCanceled();
  }
 
  // Checks: the `msg.sender` is the creator if dealing with a transfer-based invoice
+ // or a linear/tranched stream-based invoice which was not paid yet (not streaming)
  //
  // Notes:
- // - for a linear or tranched stream-based invoice, the `msg.sender` is checked in the
+ // - Once a linear or tranched stream is created, the `msg.sender` is checked in the
  // {SablierV2Lockup} `cancel` method
- if (invoice.payment.method == Types.Method.Transfer) {
+ if (invoice.payment.method == Types.Method.Transfer || invoice.status == Types.Status.Pending) {
  if (invoice.recipient != msg.sender) {
  revert Errors.InvoiceOwnerUnauthorized();
  }
  }
-
  // Effects: cancel the stream accordingly depending on its type
- if (invoice.payment.method == Types.Method.LinearStream) {
- cancelLinearStream({ streamId: invoice.payment.streamId });
- } else if (invoice.payment.method == Types.Method.TranchedStream) {
- cancelTranchedStream({ streamId: invoice.payment.streamId });
+ //
+ // Notes:
+ // - A transfer-based invoice can be canceled directly
+ // - A linear or tranched stream MUST be canceled by calling the `cancel` method on the according
+ // {ISablierV2Lockup} contract
+ else if (invoice.status == Types.Status.Ongoing) {
+ if (invoice.payment.method == Types.Method.LinearStream) {
+ cancelLinearStream({ streamId: invoice.payment.streamId });
+ } else if (invoice.payment.method == Types.Method.TranchedStream) {
+ cancelTranchedStream({ streamId: invoice.payment.streamId });
+ }
  }
-
  // Effects: mark the invoice as canceled
  _invoices[id].status = Types.Status.Canceled;
 

src/modules/invoice-module/interfaces/IInvoiceModule.sol

@@ -72,6 +72,9 @@ interface IInvoiceModule {
  /// @notice Cancels the `id` invoice
  ///
  /// Notes:
+ /// - A transfer-based invoice can be canceled only by its creator (recipient)
+ /// - A linear/tranched stream-based invoice can be canceled by its creator only if its
+ /// status is `Pending`; otherwise only the stream sender can cancel it
  /// - if the invoice has a linear or tranched stream payment method, the streaming flow will be
  /// stopped and the remaining funds will be refunded to the stream payer
  ///

src/modules/invoice-module/libraries/Errors.sol

@@ -55,7 +55,10 @@ library Errors {
  error CannotCancelPaidInvoice();
 
  /// @notice Thrown when an attempt is made to cancel an already canceled invoice
- error CannotCancelCanceledInvoice();
+ error InvoiceAlreadyCanceled();
+
+ /// @notice Thrown when the caller is not the initial stream sender
+ error OnlyInitialStreamSender(address initialSender);
 
  /*//////////////////////////////////////////////////////////////////////////
  STREAM-MANAGER

src/modules/invoice-module/sablier-v2/StreamManager.sol

@@ -35,6 +35,13 @@ contract StreamManager is IStreamManager {
  /// @inheritdoc IStreamManager
  UD60x18 public override brokerFee;
 
+ /*//////////////////////////////////////////////////////////////////////////
+ PRIVATE STORAGE
+ //////////////////////////////////////////////////////////////////////////*/
+
+ /// @dev Stores the initial address of the account that started the stream
+ mapping(uint256 streamId => address initialSender) private _initialStreamSender;
+
  /*//////////////////////////////////////////////////////////////////////////
  CONSTRUCTOR
  //////////////////////////////////////////////////////////////////////////*/
@@ -61,6 +68,13 @@ contract StreamManager is IStreamManager {
  _;
  }
 
+ /// @notice Reverts if the `msg.sender` is not the initial stream sender (creator of the stream)
+ modifier onlyInitialStreamSender(uint256 streamId) {
+ address initialSender = _initialStreamSender[streamId];
+ if (msg.sender != initialSender) revert Errors.OnlyInitialStreamSender(initialSender);
+ _;
+ }
+
  /*//////////////////////////////////////////////////////////////////////////
  NON-CONSTANT FUNCTIONS
  //////////////////////////////////////////////////////////////////////////*/
@@ -78,6 +92,9 @@ contract StreamManager is IStreamManager {
 
  // Create the Lockup Linear stream
  streamId = _createLinearStream(asset, totalAmount, startTime, endTime, recipient);
+
+ // Set `msg.sender` as the initial stream sender to allow secure stream management
+ _initialStreamSender[streamId] = msg.sender;
  }
 
  /// @inheritdoc IStreamManager
@@ -94,6 +111,9 @@ contract StreamManager is IStreamManager {
 
  // Create the Lockup Linear stream
  streamId = _createTranchedStream(asset, totalAmount, startTime, recipient, numberOfTranches, recurrence);
+
+ // Set `msg.sender` as the initial stream sender to allow secure stream management
+ _initialStreamSender[streamId] = msg.sender;
  }
 
  /// @inheritdoc IStreamManager
@@ -165,7 +185,7 @@ contract StreamManager is IStreamManager {
  LockupLinear.CreateWithTimestamps memory params;
 
  // Declare the function parameters
- params.sender = msg.sender; // The sender will be able to cancel the stream
+ params.sender = address(this); // The sender will be able to cancel the stream
  params.recipient = recipient; // The recipient of the streamed assets
  params.totalAmount = totalAmount; // Total amount is the amount inclusive of all fees
  params.asset = asset; // The streaming asset
@@ -193,7 +213,7 @@ contract StreamManager is IStreamManager {
  LockupTranched.CreateWithTimestamps memory params;
 
  // Declare the function parameters
- params.sender = msg.sender; // The sender will be able to cancel the stream
+ params.sender = address(this); // The sender will be able to cancel the stream
  params.recipient = recipient; // The recipient of the streamed assets
  params.totalAmount = totalAmount; // Total amount is the amount inclusive of all fees
  params.asset = asset; // The streaming asset
@@ -227,12 +247,17 @@ contract StreamManager is IStreamManager {
  }
 
  /// @dev Withdraws from either a linear or tranched stream
- function _withdrawStream(ISablierV2Lockup sablier, uint256 streamId, address to, uint128 amount) internal {
+ function _withdrawStream(
+ ISablierV2Lockup sablier,
+ uint256 streamId,
+ address to,
+ uint128 amount
+ ) internal onlyInitialStreamSender(streamId) {
  sablier.withdraw(streamId, to, amount);
  }
 
  /// @dev Cancels the `streamId` stream
- function _cancelStream(ISablierV2Lockup sablier, uint256 streamId) internal {
+ function _cancelStream(ISablierV2Lockup sablier, uint256 streamId) internal onlyInitialStreamSender(streamId) {
  sablier.cancel(streamId);
  }