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

runtime-api: remove redundant version checks #7610

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

Conversation

sw10pa
Copy link
Member

@sw10pa sw10pa commented Feb 18, 2025

Issue

Description

This PR removes unnecessary runtime API version checks from subsystem-util/src/runtime for APIs supported by Polkadot (the most recent network to upgrade). Specifically, it applies to the DisabledValidators, MinimumBackingVotes and NodeFeatures APIs.

@sw10pa sw10pa requested review from tdimitrov and alindima February 18, 2025 15:01
@sw10pa sw10pa self-assigned this Feb 18, 2025
@sw10pa sw10pa added T4-runtime_API This PR/Issue is related to runtime APIs. T0-node This PR/Issue is related to the topic “node”. labels Feb 18, 2025
@sw10pa sw10pa marked this pull request as ready for review February 18, 2025 15:22
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/13396130678
Failed job name: test-linux-stable-no-try-runtime

}

/// Request the node features enabled in the runtime.
/// Pass in the session index for caching purposes, as it should only change on session boundaries.
/// Prior to runtime API version 9, just return `None`.
pub async fn request_node_features(
parent: Hash,
session_index: SessionIndex,
sender: &mut impl overseer::SubsystemSender<RuntimeApiMessage>,
) -> Result<Option<NodeFeatures>> {
Copy link
Member

Choose a reason for hiding this comment

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

This now always returns Some, so you can remove the Option.

@@ -469,61 +469,35 @@ where
}

/// Request the min backing votes value.
/// Prior to runtime API version 6, just return a hardcoded constant.
pub async fn request_min_backing_votes(
Copy link
Contributor

Choose a reason for hiding this comment

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

we can remove these helpers altogether and implement them with specialize_requests like we do for the other runtime APIs

let disabled_validators = get_disabled_validators_with_fallback(sender, parent)
.await
.map_err(|e| Error::try_from(e).expect("the conversion is infallible; qed"))?;
let disabled_validators = request_disabled_validators(parent, sender).await.await??;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: You can move request_disabled_validators call to the try_join above for consistency (like it's done for validators).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”. T4-runtime_API This PR/Issue is related to runtime APIs.
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

Cleanup boilerplate code when DisabledValidators runtime api call is released
4 participants