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

More StreamPayment config #878

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

nanocryk
Copy link
Contributor

@nanocryk nanocryk commented Feb 18, 2025

Adds 2 new config items:

  • minimum_request_deadline_delay: currently stream parties can create a mandatory change request with an immediate/very close deadline, which can allow for the payer to stop paying immediately. This new item allows to set a minimal delay (in the stream time unit) so that the other party have fair time to respond.
  • soft_minimum_deposit: Allows to configure a minimum amount of deposit when the stream source change it manually. It also prevents the source to close the stream if not stalled. This allows to guarantee to the target that payment will still occur for at least soft_minimum_deposit / rate time, and prevents the stream to be closed/emptied by surprise. This can be useful for non-automated services that could need time to prepare and react to the end of the service.

Breaking changes

This changes the StreamConfig struct stored in this pallet but also in DataPreservers pallets as a config can be stored in a profile if the preserver wishes to provide services in exchange for a specific stream payment.

TODOs:

  • Write migration

@nanocryk nanocryk added a0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Feb 18, 2025
Copy link
Contributor

github-actions bot commented Feb 18, 2025

WASM runtime size check:

Compared to target branch

dancebox runtime: 1428 KB (no changes) ✅

flashbox runtime: 840 KB (no changes) ✅

dancelight runtime: 2188 KB (no changes) ✅

container chain template simple runtime: 1132 KB (no changes) ✅

container chain template frontier runtime: 1416 KB (no changes) ✅

Copy link
Contributor

github-actions bot commented Feb 18, 2025

Coverage Report

(master)

@@                          Coverage Diff                           @@
##           master   jeremy-more-stream-payment-configs      +/-   ##
======================================================================
+ Coverage   65.40%                               65.57%   +0.17%     
+ Files         350                                  351       +1     
+ Lines       60856                                61165     +309     
======================================================================
+ Hits        39798                                40105     +307     
+ Misses      21058                                21060       +2     
Files Changed Coverage
/chains/orchestrator-paras/runtime/common/src/migrations.rs 85.92% (+0.36%)
/chains/orchestrator-paras/runtime/dancebox/src/lib.rs 88.06% (+0.07%)
/chains/orchestrator-paras/runtime/dancebox/src/tests/integration_test.rs 99.71% (+0.01%)
/chains/orchestrator-paras/runtime/flashbox/src/lib.rs 46.95% (+0.10%)
/chains/orchestrator-paras/runtime/flashbox/src/tests/integration_test.rs 99.84% (+0.01%)
/pallets/stream-payment/src/lib.rs 88.29% (-0.11%)

Coverage generated Thu Feb 27 14:35:26 UTC 2025

Copy link
Contributor

@girazoki girazoki left a comment

Choose a reason for hiding this comment

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

Minor comment about try-runtime, I think it's a good excercise that we always write those hooks

@girazoki girazoki self-requested a review February 25, 2025 09:18
Copy link
Contributor

@girazoki girazoki left a comment

Choose a reason for hiding this comment

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

IMportant commens of formatting and question whether we should include both in dancebox and flashbox

Copy link
Contributor

@girazoki girazoki left a comment

Choose a reason for hiding this comment

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

looks good now @nanocryk

@nanocryk nanocryk added the A8-mergeoncegreen Pull request is reviewed well. label Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a0-pleasereview Pull request needs code review. A8-mergeoncegreen Pull request is reviewed well. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants