Merge the "withdraw" with the "withdraw to" functions

[PaulRBerg]

There's quite a bit of repetitive code in the current withdraw functions, which is mainly due to the plurality of functions we have.

What if we merged the basic "withdraw" and the "withdraw to" variants, like this?

• We would only have the withdraw and withdrawAll functions, but each would have an additional argument to
• to can be any other address, but only if the caller is the recipient or an approved operator thereof
• If the caller is the sender but to is not the recipient, the function reverts

This design would let us delete two functions and two large test suites, with the only cost being the slight added cost for checking the new input argument requirements.

[andreivladbrg]

I like the idea. The code reduction would be nice.

[PaulRBerg]

I'm glad that you like it. Purging these functions might actually be necessary - as I have just seen in this CI report, the forge build --sizes command failed in PR #193 due to the SablierV2Pro contract having gone above the 24kB size limit.

[andreivladbrg]

Is the to address check still necessary in the withdraw function? The ERC20 transfer function would revert with this reason.

[PaulRBerg]

Good catch. I don't think we need to keep the check if it is also implemented in the ERC-20 contract itself. However, I would like us to keep the test (and update the custom errors, accordingly), because we should definitely forbid withdrawals to the zero address.

[PaulRBerg]

Locking, implemented in #205.