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

ServicesPayment in Starlight runtime #649

Merged
merged 6 commits into from
Aug 13, 2024

Conversation

fgamundi
Copy link
Contributor

@fgamundi fgamundi commented Aug 7, 2024

Adds ServicePayment pallet to Starlight runtime. This also enables service charging on collator assignment, free credits on registration and removal of assignment on insufficient credits.

Copy link
Contributor

github-actions bot commented Aug 7, 2024

Coverage Report

(master)

@@                        Coverage Diff                        @@
##           master   fg-services-payment-starlight      +/-   ##
=================================================================
+ Coverage   66.11%                          66.19%   +0.08%     
+ Files         263                             264       +1     
+ Lines       45754                           45952     +198     
=================================================================
+ Hits        30246                           30414     +168     
+ Misses      15508                           15538      +30     
Files Changed Coverage
/primitives/traits/src/lib.rs 71.59% (-11.36%) 🔽
/solo-chains/runtime/starlight/src/genesis_config_presets.rs 23.20% (-0.36%) 🔽
/solo-chains/runtime/starlight/src/lib.rs 24.63% (+2.38%) 🔼
/solo-chains/runtime/starlight/src/tests/common/mod.rs 96.09% (+0.15%) 🔼

Coverage generated Fri Aug 9 13:45:01 UTC 2024

@fgamundi fgamundi changed the title ServicesPayment in Starlight and integration tests ServicesPayment in Starlight runtime Aug 7, 2024
@fgamundi fgamundi added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Aug 7, 2024
@fgamundi fgamundi marked this pull request as ready for review August 7, 2024 19:35
@tmpolaczyk
Copy link
Contributor

zombie_tanssi_relay tests are failing because parachain 1000 is not producing blocks. Maybe it doesn't have credits?

@fgamundi
Copy link
Contributor Author

fgamundi commented Aug 8, 2024

@tmpolaczyk it's passing now

```bash
STARLIGHT_EPOCH_DURATION=10 ./polkadot/scripts/build-only-wasm.sh starlight-runtime /path/to/output/directory/
./polkadot/scripts/build-only-wasm.sh starlight-runtime /path/to/output/directory/
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this so EpochDurationInBlocks could be a const, so FreeCollatorAssignmentCredits could also be a const. It wasn't needed, but I didn't see any place where we were using this env variable that came from Rococo.

XcmCoreBuyer::para_deregistered(para_id);
*/
ServicesPayment::para_deregistered(para_id);

Weight::default()
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems we are not using this returned weight. Plus the pallet specific handlers also not returning Weight, Can we remove this in future PRs?

Copy link
Contributor

@ParthDesai ParthDesai left a comment

Choose a reason for hiding this comment

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

One minor comment which is not blocking.

Rest looks good.

@ParthDesai ParthDesai merged commit 370b748 into master Aug 13, 2024
37 checks passed
@ParthDesai ParthDesai deleted the fg-services-payment-starlight branch August 13, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants