Dealing with refunds of maximum amount

[smol-ninja]

Context

@razgraf pointed out in the Slack that refund would very likely revert if it's called with the refundableAmountOf. The refundableAmountOf returns the maximum amount that a user can withdraw, and by the time transaction is mined in the block, this amount would have already exceeded the maximum amount that can be refunded at the block timestamp (due to streaming). This makes it impossible for a user to withdraw the max amount until dealing with a paused stream.

batch can also not be used in this case since there is no way to enter the returned value of one function as the input parameter of another. We will think about improving batch in the future release. But for now three simple solutions have been discussed on Slack to resolve this.

Solutions

Solution 1

Implement refundMax that implicitly calculates the max refundable amount.

Solution 2

Replace amount input with a percentage in refund function. So if 50% is provided as input, a refund of 50% of the max refundable will be processed.

Solution 3

The refund can be modified to allow maxAmount instead of amount to signal how much maximum the user wants to refund. So, in cases where the maximum refundable amount goes down by the time the tx is included in a block, the function would still make a refund but with the max amount at that time.

These two solutions have been discussed on the Slack. Feel free to share more ideas. cc @sablier-labs/engineers

[PaulRBerg]

Huh — this is an interesting case where end-to-end product integration uncovered a UX issue that would have been difficult to spot otherwise.

Solution 1 seems the most straightforward. Let's go with that.

There might be cases where users want to refund a particular amount for accounting reasons, e.g., they made a big deposit accidentally and wish to receive a portion back.

Also, having two discrete functions refund and refundMax is helpful and clearer than a single refund function with more convoluted behavior.

[smol-ninja]

There might be cases where users want to refund a particular amount for accounting reasons, e.g., they made a big deposit accidentally and wish to receive a portion back.

If you think solution 3 doesn't achieve that, let me correct it. In solution 3, if amount provided exceeds maximum amount then it refunds the max amount instead of reverting. Otherwise it refunds the amount provided.

I agree with the rest of your points.

[PaulRBerg]

I was referring to solution 2 (which doesn't achieve that), but thanks for clarifying.

I prefer solution 1 over solution 3 because the former is more straightforward.

[andreivladbrg]

my vote is for refundMax

[PaulRBerg]

Closing as per a majority vote

[razgraf]

Solution 2 would be the one that's the least prone to errors.

Its downside, as mentioned on slack is the user-facing form UX (we'd present the user with a percentage slider rather than an actual amount input). PRBs point re. accounting reasons and Mircea's take re. consistency with our other forms is what makes me wonder if it's the best choice...

It covers all the bad cases execution-wise, but doesn't offer the best UX.

Solution 1 is an easy win. I think most cases will be of people wanting to "rage quit" a.k.a. void + withdraw + refund, which requires withdrawMax and refundMax. For the other cases where specific amounts are selected at refund, we'll have to warn the user (e.g. if the amount selected is prone to a revert if not executed in the next 24 hours, we warn you about it). Not ideal (Solution 1 would hide this warning circus) but should be easy enough to understand for the user.

Solution 3 is a nice mix between not-reverting and having a refundMax for rage quits. But it would, as PRB mentioned, hide a bit too much logic underneath. I think we've discussed a similar design for withdrawMax when we were doing Lockup but decided against it for similar reasons.

[PaulRBerg]

Great.

Yes, we decided to implement withdrawMax for the same reasons (simplicity and security).

[andreivladbrg]

"rage quit" a.k.a. void + withdraw + refund

just a note on this: perfect use case for batch function (recipient would also receive the funds)