Redesign the "to" parameter in the proxy target

[PaulRBerg]

Re-posting @razgraf from Slack:

Looking at withdraw multiple for the target and realised the to parameter doesn't really make sense.

What if I as a proxy sender want to withdraw from multiple streams for my employees. The to parameter, because I am a sender, should be locked to each stream's recipient. Unfortunately, I can only provide the to parameter as one single recipient address. So I'm limited to withdrawing multiple streams only between the same sender + recipient pair, if I am the sender. Kind of blocks the possibilities even for relayers right? At least for my use-case above with a company and its employees.
I don't remember if we intended to to this. Am I interpreting the code correctly?

At the very least, if this is indeed the problem, can we maybe re-code the withdrawMultiple method in the new targets to loop around an array of structs of {streamIds , to] instead of lock to a single possible to?

[PaulRBerg]

It's a good observation.

I don't remember if we intended to to this. Am I interpreting the code correctly?

The withdrawMultiple is first and foremost designed to be a helper for recipients. We squeezed in the sender's ability to withdraw shortly before wrapping up the contracts for audit.

Also, for context, we added support for recipient methods in the proxy target just in case anyone ends up using it as a Sablier recipient. See the related discussion in https://github.com/cantinasec/review-sablier2/issues/9

Kind of blocks the possibilities even for relayers right?

No. Those would have system-level ability to withdraw on behalf of any stream. And the method name would be different, anyway.

an array of structs of {streamIds , to] instead of lock to a single possible to?

That's one way to solve it, yes.

The other way is to write a dedicated method meant to be called exclusively by senders, and query the recipients at runtime—less gas-efficient but better UX.

However, all this will be unnecessary given that we have decided to remove the sender's ability to withdraw:sablier-labs/v2-core#674

[andreivladbrg]

I don’t think it’s a good idea because it would require duplicating all withdraw functions (except for withdrawMaxAndTransfer), leading to an increase in the contract sizes. Without duplication, for example, with withdrawMultiple, things would get complicated when a sender wants to withdraw for multiple different recipients.

[PaulRBerg]

I don't follow your logic, @andreivladbrg.

The issue unsurfaced by @razgraf pertains to the withdrawMultiple function in particular; there's no problem whatsoever with withdraw.

An increase in the contract sizes should be acceptable, and I am still in favor of implementing a new withdrawMultiple.

[PaulRBerg]

Any additional thoughts on this, @andreivladbrg?

[andreivladbrg]

Oh, I just realized that I misunderstood the point because of this: "I am inclined to go with implementing a dedicated function for senders."

It made me think that you were also suggesting having different functions in v2-core for the sender and for the recipient, I was against this idea. However I've changed my mind now, because I realized that if we have separate functions for the sender and for the recipient (implicitly for approved operators) in core, it would allow the recipients to use ERC721.approve and let the sender withdraw to any address for them. What do you think? Should we consider this?

Regarding the suggestion, yes, I am now also in favor.

[PaulRBerg]

Thanks Andrei. I have created an issue: #215

It made me think that you were also suggesting having different functions in v2-core for the sender and for the recipient

No, I have never suggested that, but it is an interesting idea.

What do you think? Should we consider this?

Yes, but this discussion is not about V2 Core. Could you please open a separate discussion there?

[PaulRBerg]

Closing since we have decided to get rid of the proxy architecture in V2.1