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

Enable async backing on Kusama #90

Closed

Conversation

Sophia-Gold
Copy link

@Sophia-Gold Sophia-Gold commented Nov 15, 2023

This enables async backing in the Kusama runtime. Note that for candidates to actually be backed asynchronously, a subsequent governance proposal changing two new parameters is needed.

@Sophia-Gold Sophia-Gold mentioned this pull request Nov 16, 2023
@Sophia-Gold Sophia-Gold marked this pull request as ready for review November 21, 2023 19:37
Copy link
Contributor

@eskimor eskimor 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, although I think we should now also bump the version of the trait to (7) - at least once we have this on Polkadot as well. @tdimitrov ?

@@ -2040,6 +2041,18 @@ sp_api::impl_runtime_apis! {
key_ownership_proof,
)
}

fn minimum_backing_votes() -> u32 {
parachains_runtime_api_impl::minimum_backing_votes::<Runtime>()
Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting seems off.

Copy link
Author

Choose a reason for hiding this comment

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

Done in 9f5cef4. Not sure why this wasn't caught by rustfmt.

Bullrich and others added 2 commits November 22, 2023 15:32
When adapted the code from polkadot-fellows#88 I forgot to update the location of the
credentials, so it was using review-bot which can only enable auto-merge
but can not merge a ready to merge PR. Follow up to polkadot-fellows#97
relay/kusama/src/lib.rs Outdated Show resolved Hide resolved
@tdimitrov
Copy link
Contributor

Looks good, although I think we should now also bump the version of the trait to (7) - at least once we have this on Polkadot as well. @tdimitrov ?

Yes, but it is already done?
https://github.com/polkadot-fellows/runtimes/pull/90/files#diff-efa4caeb17487ecb13d8f5eb7863c3241d84afa2e73fbf25909a2ca89df0f362R1913

@Sophia-Gold
Copy link
Author

Looks good, although I think we should now also bump the version of the trait to (7) - at least once we have this on Polkadot as well. @tdimitrov ?

I assume you mean 8?

Co-authored-by: Tsvetomir Dimitrov <[email protected]>
@eskimor
Copy link
Contributor

eskimor commented Nov 23, 2023

Yes, but it is already done?

No. I meant the trait itself, but this can only be done once we also launched on Polkadot: So disregard.

Copy link
Contributor

@ordian ordian left a comment

Choose a reason for hiding this comment

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

Expect chaos.

Copy link
Contributor

@joepetrowski joepetrowski left a comment

Choose a reason for hiding this comment

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

How about enabling system parachains to use it?

Copy link
Member

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

lgtm but this is also done in #87

@Sophia-Gold
Copy link
Author

How about enabling system parachains to use it?

They won't be able to make use of it until we propose an update to the new parameters after this is enacted.

Glutton is already upgraded in Polkadot-SDK. I'll move that here. Can your team upgrade the others?

@joepetrowski
Copy link
Contributor

Glutton is already upgraded in Polkadot-SDK. I'll move that here. Can your team upgrade the others?

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants