Allow stream senders to cancel/renounce through a Stream Management contract

[gabrielstoica]

Hi,

As stated here, if a Stream Management contract is used, any stream sender should be allowed to cancel/renounce it. However, the msg.sender must be the stream's sender according to this checker.

Therefore, any call made by a stream sender through a Stream Management contract to the cancel or renounce method will result in a SablierV2Lockup_Unauthorized revert because the msg.sender is not the stream's sender but rather the Stream Management itself.

How should one allow stream senders to cancel or renounce it directly through a Stream Management-like contract?

Thanks!

[andreivladbrg]

Hi Gabriel, thanks for the question.

Therefore, any call made by a stream sender through a Stream Management contract to the cancel or renounce method will result in a SablierV2Lockup_Unauthorized revert because the msg.sender is not the stream's sender but rather the Stream Management itself.

This is correct.

How should one allow stream senders to cancel or renounce it directly through a Stream Management-like contract?

The solution to this is to pass the Stream Management contract address as the sender parameter in the create function before creating a stream. Lmk if this answers your question.

P.S. since this is more of a question, it is more suited to be in the discussion section rather than an issue. So, I will convert it.

[gabrielstoica]

Hi Andrei,

Thanks for your feedback! Your proposal might be a solution but it would mean that all streams created through the Stream Management will be displayed on the front-end side with the contract address as the sender rather than the end user's one, right?

As you can see, this approach has its downsides, as in my case I want to create a stream through the Stream Management as part of multiple actions and want to keep the user's address as the sender.

One solution I can see here is to add a mapping in the Stream Management keeping track of each stream "created" on the user's behalf, but still is not the natural approach.

Have you considered checking for the tx.origin rather than msg.sender in the _isCallerStreamSender method? I appreciate this option might come with some security considerations.

[andreivladbrg]

all streams created through the Stream Management will be displayed on the front-end side with the contract address as the sender rather than the end user's one, right

It doesn’t matter who is creating the stream, but what parameter you pass as the sender. So, if you pass the Stream Management contract, in the front-end, it will be the sender.

the Stream Management as part of multiple actions and want to keep the user's address as the sender

I am curious, why do you want to keep the user’s address as the sender, and at the same time have a contract that automates the cancel/renounce.

add a mapping in the Stream Management keeping track of each stream "created" on the user's behalf,

sounds like a solution

Have you considered checking for the tx.origin rather than msg.sender in the _isCallerStreamSender method

Using tx.origin is considered a bad practice in Ethereum, so it has its security downsides, for example see this

[gabrielstoica]

I am curious, why do you want to keep the user’s address as the sender, and at the same time have a contract that automates the cancel/renounce.

My concrete use-case involves creating an on-chain invoice that can be paid via a stream. There's an InvoiceManager contract that inherits from StreamManagement automating the stream creation on the payer's behalf for a stream-based invoice.

Therefore, I want to display all the payers streams on the front-end side even though they were created through this management contract. In this case, since the sender is the invoice payer, the cancel/renounce process cannot be automated through the InvoiceManager when i.e. an invoice is canceled.

Using tx.origin is considered a bad practice in Ethereum, so it has its security downsides, for example see this

I'm aware of this security consideration and I appreciate your feedback.

[smol-ninja]

Hi @gabrielstoica, thanks for explaining your use case. This helps us understand how we can resolve this issue for you. I have a few questions and would appreciate your answers:

1. Who decides when to cancel the invoice? Is it sender or InvoiceManager?

2. Since the sender is the invoice payer, why shouldn't the sender be allowed to cancel the stream? If InvoiceManager can take action on behalf of the sender, can't it be misused so that the manager cancels the invoice without the sender's consent?

3. If there is a consent mechanism in place, then as part of that consent, you can allow the sender to cancel the stream through a UI action. I am sure you have thought about it but curious if there is any issue with this approach?

[gabrielstoica]

Hi @smol-ninja, thanks for your feedback and interest in hearing more.

Who decides when to cancel the invoice? Is it sender or InvoiceManager?

The sender decides when to cancel the invoice. Invoice cancellation means (among other things) that the payer no longer wants to stream any funds to the payee.

Since the sender is the invoice payer, why shouldn't the sender be allowed to cancel the stream?

You're right, the sender should be allowed to cancel the stream, but in the current implementation, since the sender is different than the InvoiceManager it will not be possible to cancel the invoice through the InvoiceManager contract and therefore extra logic must be considered.

If InvoiceManager can take action on behalf of the sender, can't it be misused so that the manager cancels the invoice without the sender's consent?

This cannot be possible as there is a parent contract invoking InvoiceManager's methods only after proper checks are passed.

If there is a consent mechanism in place, then as part of that consent, you can allow the sender to cancel the stream through a UI action. I am sure you have thought about it but curious if there is any issue with this approach?

The stream cancellation is just one part of the invoice cancellation, so multiple actions might take place. In the best scenario all these operations should be atomic. Allowing the sender to cancel the stream through a UI action results in having two transactions: 1 - stream cancellation, 2 - invoice cancellation.

[gabrielstoica]

Hi @andreivladbrg, @smol-ninja!

I've found a workaround for this that solves the "problem".

In the StreamManager contract I've added a mapping called _initialStreamSender defined as follows:

mapping(uint256 streamId => address initialSender) private _initialStreamSender;

When a new stream gets created, the sender will be the InvoiceManager and the _initialStreamSender[streamId] will be set to msg.sender which is the payer of the invoice. This will guard the cancel method call and allow only the right user to cancel a stream.

On the front-end side, there is an event that logs the payment of an invoice together with the initial stream sender so we can display all invoices (+ their streams) with regard to their payers.

Thanks for your support!

[smol-ninja]

Incredible! That's a neat solution, @gabrielstoica and we look forward to seeing it in action. Let us know if there are any updates or further questions.

[PaulRBerg]

Neat. Maybe we should reference this design in the integration template/ example repos, @smol-ninja, @andreivladbrg?

Thank you for opening this discussion @gabrielstoica.

[gabrielstoica]

Thanks, @PaulRBerg - it's a pleasure to give a hand!
I've just open-sourced our repository in case you want to see it in action. Check the implementation here.

Any feedback would be much appreciated.