Streaming payments - preventing loss of income on uninstall

[arrenv]

Description

Currently, the streaming payments extension allows the extension to be installed terminating all streams and owed funds regardless if there are active streams or funds have been earned by recipients but are unclaimed.

This poses a risk to recipients, presents an opportunity for bad actors who could clear obligations, or mistakes that could lead to a loss of expected and owed funds to recipients.

Changes

To rectify this, we should safeguard the uninstalling of the streaming payment extensions if there are unclaimed funds and/or there are active streams. Specifically:

• Introduce checks to detect the presence of active streams and unclaimed funds.
• If active streams exist, revert the transaction with a message indicating the presence of active streams.
• If unclaimed funds are present, revert the transaction with an appropriate message.

[kronosapiens]

Hey @arrenv,

A few thoughts:

• The way we currently track whether a payment has "ended" is by saving an endTime for all payments. If we are before the end time, the payment is active. If we are after it, is has ended. I do not think it is worth distinguishing between "active streams" and "unclaimed funds" -- they are the same thing.
• The simplest way to address this would be to track the globalEndTime for all payments, and disallow uninstalling the extension unless the globalEndTime has elapsed, potentially with an additional 3 or 7 day grace period.
• Note that we cannot guarantee that funds will be available in the funding pot to cover a streaming payment -- it is up to the colony to ensure that this funding is available. That is a constraint that has always existed and affects all our payment options.

[arrenv]

@kronosapiens Thank you for looking over it.

• The stream could have passed the endTime and "ended", but the recipient could still not have claimed their funds, right?
• A globalEndTime makes sense for knowing if there are streams that have not "ended". We would need to have a "globalUnclaimed" as well though, right? The difference between what has "streamed" and what has been claimed.
• Yes, this is expected. I would say that it would be better to show that the colony has unpaid liabilities that are not able to be paid then to allow it to clear those liabilities out, purposefully or not.

[kronosapiens]

@arrenv You are right in that a stream could have ended while still being unclaimed by the user. This is why I suggested a 3 or 7 day grace period in which the user can claim any available funds before the extension can be uninstalled.

I do not think that tracking the unclaimed funds directly will be beneficial, as this creates a potential griefing exploit where a user deliberately avoids claiming funds to prevent the colony from uninstalling the extension. Using a grace period after the end time avoids this problem.

Edit: I checked the implementation and anyone can "claim" the stream on behalf of the recipient, so that exploit is less impactful than I had thought. I still don't think it is worth tracking at that level of granularity -- I could imagine situations with many small, unclaimed streams gumming up the works, forcing a colony to do a lot of work to clear them up before uninstalling (for example, if streaming a zero-value token for marketing purposes to thousands of users). A grace period would allow colonies to exit these types of situations much more cleanly, while still insuring users aren't being denied their funds.

[arrenv]

@kronosapiens The grace period makes sense and seems reasonable, the limitation I see though is that if there are no funds to claim, then even in the grace period you could not claim and then all liabilities would essentially be erased.

[kronosapiens]

@arrenv sure, that's a conceivable risk. But ultimately I would say that if a colony wants to withhold funds, then there's no way to force them to provide them. Having the contract be uninstallable won't make a difference in that regard. I would prioritize simplicity of implementation and reasonable precaution.

[arrenv]

Alex raised that the contract can be simplified by removing multiple tokens with a single stream, which we are not using in the frontend anyway. Does this align better and make it more reasonable?