-
Notifications
You must be signed in to change notification settings - Fork 732
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
Coretime: Add request revenue info #3940
Conversation
let value = | ||
Balances::reducible_balance(&stash, Preservation::Expendable, Fortitude::Polite); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably just take all of it, but not that important here for the test chain.
Co-authored-by: Bastian Köcher <[email protected]>
|
||
message.push(mk_coretime_call::<T>(CoretimeCalls::NotifyRevenue((when, raw_revenue)))); | ||
|
||
send_xcm::<T::SendXcm>(dest.clone(), Xcm(message))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We know that this will send with Root privilege? It will be necessary for the UnpaidExecution
instruction. IMO this is where testing with xcm-emulator is worth it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it works in manual zombienet tests and in integration tests, so I guess it's likely to be okay... But if someone could make it through the emulator that would be great!
cumulus/parachains/runtimes/coretime/coretime-rococo/src/coretime.rs
Outdated
Show resolved
Hide resolved
The CI pipeline was cancelled due to failure one of the required jobs. |
How do i integrate this into a runtime? PRdoc and or MR description should provide some integration advise for downstream teams. |
Well, it's a fair point, although I doubt the PRdoc is a good place for it... Anyway, I was going to start a PR in the fellowship repo to integrate it into Kusama first. |
Enables the `request_revenue` and `notify_revenue` parts of [RFC 5 - Coretime Interface](https://polkadot-fellows.github.io/RFCs/approved/0005-coretime-interface.html) TODO: - [x] Finish first pass at implementation - [x] ~~Need to explicitly burn uncollected and dropped revenue~~ Accumulate it instead - [x] Confirm working on zombienet - [x] Tests - [ ] Enable XCM `request_revenue` sending on Coretime chain on Kusama and Polkadot Fixes: paritytech#2209 --------- Co-authored-by: Dmitry Sinyavin <[email protected]> Co-authored-by: command-bot <> Co-authored-by: s0me0ne-unkn0wn <[email protected]> Co-authored-by: Dónal Murray <[email protected]> Co-authored-by: Bastian Köcher <[email protected]>
@@ -22200,34 +22185,31 @@ dependencies = [ | |||
|
|||
[[package]] | |||
name = "time" | |||
version = "0.3.36" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why time
upgrade had to be reverted, but it created compilation issues that was fixed in #4862, so stable2407
is broken on nightly again. Does no one review lock file changes or what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that was an oversight. Downstream users should not have the problem as they can just upgrade the patch version. When you compile the stable in the repo, there is no other way around than bumping this locally for you.
Enables the
request_revenue
andnotify_revenue
parts of RFC 5 - Coretime InterfaceTODO:
Need to explicitly burn uncollected and dropped revenueAccumulate it insteadrequest_revenue
sending on Coretime chain on Kusama and PolkadotFixes: #2209