Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Streaming payments only support a single stream #1250

Merged
merged 4 commits into from
Jun 10, 2024

Conversation

area
Copy link
Member

@area area commented May 8, 2024

NB Based on maint/pnpm.

Multiple streams in a single streaming payment are no longer desired on the frontend, so taking the opportunity to simplify the contracts. This will also allow accurate tracking of the number of pending payments, allowing the 'prevent uninstall if pending payments' feature.

I've taken advantage of finishUpgrade to make sure someone can't upgrade from v4 to v5, which we are declaring is impossible due to the storage layout changes.

Copy link
Member

@kronosapiens kronosapiens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. I still don't love all the extra logic about pseudo claims, etc which we added to allow admins to change payout rates, but that's not what this PR is about.

contracts/extensions/StreamingPayments.sol Outdated Show resolved Hide resolved
Base automatically changed from maint/pnpm to develop May 23, 2024 06:46
@area area force-pushed the maint/single-stream-payments branch 2 times, most recently from fc1cdaa to a78f8c2 Compare May 30, 2024 07:52
@area area force-pushed the maint/single-stream-payments branch 2 times, most recently from c0ee585 to 5a5ae87 Compare June 5, 2024 08:35
@area
Copy link
Member Author

area commented Jun 5, 2024

NB Now based on #1262

@area area force-pushed the maint/single-stream-payments branch from 5a5ae87 to 89fe76c Compare June 6, 2024 16:22
@kronosapiens
Copy link
Member

kronosapiens commented Jun 7, 2024

What's with all the storage file changes? Does the PR need a rebase? Also is this ready to go off draft?

@area area marked this pull request as ready for review June 10, 2024 08:24
@area
Copy link
Member Author

area commented Jun 10, 2024

It does not need rebasing. The one change in .storage-layouts-normalized is expected - we have changed a struct, and removed a mapping. The changes in .storage-layouts to other files are because of changes to the code since the last time a storage slot was changed, and so AST IDs have changed in the rest of the compiled contracts. #1258 attempted to go through this in more detail, but we can have a call to make sure we both know what's going on, if you'd prefer.

Copy link
Member

@kronosapiens kronosapiens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@kronosapiens kronosapiens merged commit 78685fa into develop Jun 10, 2024
2 checks passed
@kronosapiens kronosapiens deleted the maint/single-stream-payments branch June 10, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants