-
Notifications
You must be signed in to change notification settings - Fork 771
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
Disabled validators runtime API #1257
Changes from 6 commits
800d8e5
936216d
eaca5ed
e4dee17
0d0129c
5e7655f
6fb4b7f
70dc6b6
bca3c83
f717d0b
2a5af89
2c53894
a3bcb2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,6 +66,7 @@ pub(crate) struct RequestResultCache { | |
key_ownership_proof: | ||
LruMap<(Hash, ValidatorId), Option<vstaging::slashing::OpaqueKeyOwnershipProof>>, | ||
minimum_backing_votes: LruMap<SessionIndex, u32>, | ||
disabled_validators: LruMap<Hash, Vec<ValidatorIndex>>, | ||
|
||
staging_para_backing_state: LruMap<(Hash, ParaId), Option<vstaging::BackingState>>, | ||
staging_async_backing_params: LruMap<Hash, vstaging::AsyncBackingParams>, | ||
|
@@ -99,6 +100,7 @@ impl Default for RequestResultCache { | |
unapplied_slashes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), | ||
key_ownership_proof: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), | ||
minimum_backing_votes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), | ||
disabled_validators: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), | ||
|
||
staging_para_backing_state: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), | ||
staging_async_backing_params: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), | ||
|
@@ -448,6 +450,21 @@ impl RequestResultCache { | |
self.minimum_backing_votes.insert(session_index, minimum_backing_votes); | ||
} | ||
|
||
pub(crate) fn disabled_validators( | ||
&mut self, | ||
relay_parent: &Hash, | ||
) -> Option<&Vec<ValidatorIndex>> { | ||
self.disabled_validators.get(relay_parent).map(|v| &*v) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, what that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It converts &mut to & |
||
} | ||
|
||
pub(crate) fn cache_disabled_validators( | ||
&mut self, | ||
relay_parent: Hash, | ||
disabled_validators: Vec<ValidatorIndex>, | ||
) { | ||
self.disabled_validators.insert(relay_parent, disabled_validators); | ||
} | ||
|
||
pub(crate) fn staging_para_backing_state( | ||
&mut self, | ||
key: (Hash, ParaId), | ||
|
@@ -524,6 +541,7 @@ pub(crate) enum RequestResult { | |
vstaging::slashing::OpaqueKeyOwnershipProof, | ||
Option<()>, | ||
), | ||
DisabledValidators(Hash, Vec<ValidatorIndex>), | ||
|
||
StagingParaBackingState(Hash, ParaId, Option<vstaging::BackingState>), | ||
StagingAsyncBackingParams(Hash, vstaging::AsyncBackingParams), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ use primitives::{ | |
AsyncBackingParams, BackingState, CandidatePendingAvailability, Constraints, | ||
InboundHrmpLimitations, OutboundHrmpChannelLimitations, | ||
}, | ||
Id as ParaId, | ||
Id as ParaId, ValidatorIndex, | ||
}; | ||
use sp_std::prelude::*; | ||
|
||
|
@@ -123,3 +123,12 @@ pub fn async_backing_params<T: configuration::Config>() -> AsyncBackingParams { | |
pub fn minimum_backing_votes<T: initializer::Config>() -> u32 { | ||
<configuration::Pallet<T>>::config().minimum_backing_votes | ||
} | ||
|
||
/// Implementation for `DisabledValidators` | ||
pub fn disabled_validators<T: pallet_session::Config>() -> Vec<ValidatorIndex> { | ||
<pallet_session::Pallet<T>>::disabled_validators() | ||
Overkillus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.iter() | ||
.cloned() | ||
.map(|v| ValidatorIndex(v)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that's correct since the type |
||
.collect() | ||
} |
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.
Still not sure why we need to expose everything via RPC. I personally would limit the public facing APIs as much as possible. Anything that is exposed, we need to assume we can not change it without breaking things.
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.
The very least we should communicate clearly that these RPC calls are an unstable feature, which might change anytime.
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.
paritytech/cumulus#2160 (comment)
Well, I agree we should limit the scope for collators. But this is out of scope (hehe) of this PR.