From 73dc2e3cde3ded565aa76c96bf0dcdfab51e93d6 Mon Sep 17 00:00:00 2001 From: nanocryk <6422796+nanocryk@users.noreply.github.com> Date: Tue, 2 Jul 2024 16:29:39 +0200 Subject: [PATCH] add authorities and min_slot_freq fn to interface (#25) * add authorities and min_slot_freq fn to interface * fix tests + clippy * fmt --- .../orchestrator-chain-interface/src/lib.rs | 44 +++++++++++++- .../src/lib.rs | 49 ++++++++++++--- .../authorities-noting/src/benchmarks.rs | 4 +- .../authorities-noting/src/tests.rs | 2 +- .../authorities-noting-inherent/src/tests.rs | 24 +++++++- pallets/xcm-executor-utils/src/benchmarks.rs | 6 +- pallets/xcm-executor-utils/src/filters.rs | 60 ++++++++----------- pallets/xcm-executor-utils/src/migrations.rs | 26 ++++---- pallets/xcm-executor-utils/src/tests.rs | 28 +++++---- .../slot-duration-runtime-api/Cargo.toml | 1 + 10 files changed, 164 insertions(+), 80 deletions(-) diff --git a/client/orchestrator-chain-interface/src/lib.rs b/client/orchestrator-chain-interface/src/lib.rs index d9ab9e1..2b1eb0e 100644 --- a/client/orchestrator-chain-interface/src/lib.rs +++ b/client/orchestrator-chain-interface/src/lib.rs @@ -24,10 +24,13 @@ //! //! prove_read: generates a storage proof of a given set of keys at a given Orchestrator parent -pub use dp_core::{Hash as PHash, Header as PHeader}; use { - core::pin::Pin, futures::Stream, polkadot_overseer::Handle, sc_client_api::StorageProof, - sp_api::ApiError, sp_state_machine::StorageValue, std::sync::Arc, + core::pin::Pin, dp_core::ParaId, futures::Stream, polkadot_overseer::Handle, + sc_client_api::StorageProof, sp_api::ApiError, sp_state_machine::StorageValue, std::sync::Arc, +}; +pub use { + cumulus_primitives_core::relay_chain::Slot, + dp_core::{Hash as PHash, Header as PHeader}, }; #[derive(thiserror::Error, Debug)] @@ -87,6 +90,8 @@ pub type OrchestratorChainResult = Result; /// Trait that provides all necessary methods for interaction between collator and orchestrator chain. #[async_trait::async_trait] pub trait OrchestratorChainInterface: Send + Sync { + type AuthorityId; + /// Fetch a storage item by key. async fn get_storage_by_key( &self, @@ -118,6 +123,21 @@ pub trait OrchestratorChainInterface: Send + Sync { async fn finality_notification_stream( &self, ) -> OrchestratorChainResult + Send>>>; + + /// Return the set of authorities assigned to the paraId where + /// the first eligible key from the keystore is collating + async fn authorities( + &self, + orchestrator_parent: PHash, + para_id: ParaId, + ) -> OrchestratorChainResult>>; + + /// Returns the minimum slot frequency for this para id. + async fn min_slot_freq( + &self, + orchestrator_parent: PHash, + para_id: ParaId, + ) -> OrchestratorChainResult>; } #[async_trait::async_trait] @@ -125,6 +145,8 @@ impl OrchestratorChainInterface for Arc where T: OrchestratorChainInterface + ?Sized, { + type AuthorityId = T::AuthorityId; + fn overseer_handle(&self) -> OrchestratorChainResult { (**self).overseer_handle() } @@ -164,4 +186,20 @@ where ) -> OrchestratorChainResult + Send>>> { (**self).finality_notification_stream().await } + + async fn authorities( + &self, + orchestrator_parent: PHash, + para_id: ParaId, + ) -> OrchestratorChainResult>> { + (**self).authorities(orchestrator_parent, para_id).await + } + + async fn min_slot_freq( + &self, + orchestrator_parent: PHash, + para_id: ParaId, + ) -> OrchestratorChainResult> { + (**self).min_slot_freq(orchestrator_parent, para_id).await + } } diff --git a/client/orchestrator-chain-rpc-interface/src/lib.rs b/client/orchestrator-chain-rpc-interface/src/lib.rs index 12b686f..e7e110c 100644 --- a/client/orchestrator-chain-rpc-interface/src/lib.rs +++ b/client/orchestrator-chain-rpc-interface/src/lib.rs @@ -18,10 +18,12 @@ mod ws_client; use { async_trait::async_trait, - core::pin::Pin, + core::{marker::PhantomData, pin::Pin}, dc_orchestrator_chain_interface::{ - OrchestratorChainError, OrchestratorChainInterface, OrchestratorChainResult, PHash, PHeader, + OrchestratorChainError, OrchestratorChainInterface, OrchestratorChainResult, PHash, + PHeader, Slot, }, + dp_core::ParaId, futures::{Stream, StreamExt}, jsonrpsee::{core::params::ArrayParams, rpc_params}, sc_client_api::{StorageData, StorageProof}, @@ -60,11 +62,11 @@ fn url_to_string_with_port(url: Url) -> Option { )) } -pub async fn create_client_and_start_worker( +pub async fn create_client_and_start_worker( urls: Vec, task_manager: &mut TaskManager, overseer_handle: Option, -) -> OrchestratorChainResult { +) -> OrchestratorChainResult> { let urls: Vec<_> = urls .into_iter() .filter_map(url_to_string_with_port) @@ -84,18 +86,20 @@ pub async fn create_client_and_start_worker( let client = OrchestratorChainRpcClient { request_sender, overseer_handle, + _phantom: PhantomData, }; Ok(client) } #[derive(Clone)] -pub struct OrchestratorChainRpcClient { +pub struct OrchestratorChainRpcClient { request_sender: mpsc::Sender, overseer_handle: Option, + _phantom: PhantomData, } -impl OrchestratorChainRpcClient { +impl OrchestratorChainRpcClient { /// Call a call to `state_call` rpc method. pub async fn call_remote_runtime_function( &self, @@ -216,7 +220,9 @@ impl OrchestratorChainRpcClient { } #[async_trait] -impl OrchestratorChainInterface for OrchestratorChainRpcClient { +impl OrchestratorChainInterface for OrchestratorChainRpcClient { + type AuthorityId = T; + /// Fetch a storage item by key. async fn get_storage_by_key( &self, @@ -245,8 +251,8 @@ impl OrchestratorChainInterface for OrchestratorChainRpcClient { relevant_keys: &[Vec], ) -> OrchestratorChainResult { let mut cloned = Vec::new(); - cloned.extend_from_slice(&relevant_keys); - let storage_keys: Vec = cloned.into_iter().map(|key| StorageKey(key)).collect(); + cloned.extend_from_slice(relevant_keys); + let storage_keys: Vec = cloned.into_iter().map(StorageKey).collect(); self.state_get_read_proof(storage_keys, Some(orchestrator_parent)) .await @@ -281,4 +287,29 @@ impl OrchestratorChainInterface for OrchestratorChainRpcClient { let stream = tokio_stream::wrappers::ReceiverStream::new(rx); Ok(stream.boxed()) } + + /// Return the set of authorities assigned to the paraId where + /// the first eligible key from the keystore is collating + async fn authorities( + &self, + orchestrator_parent: PHash, + para_id: ParaId, + ) -> OrchestratorChainResult>> { + self.call_remote_runtime_function("para_id_authorities", orchestrator_parent, Some(para_id)) + .await + } + + /// Returns the minimum slot frequency for this para id. + async fn min_slot_freq( + &self, + orchestrator_parent: PHash, + para_id: ParaId, + ) -> OrchestratorChainResult> { + self.call_remote_runtime_function( + "parathread_slot_frequency", + orchestrator_parent, + Some(para_id), + ) + .await + } } diff --git a/container-chain-pallets/authorities-noting/src/benchmarks.rs b/container-chain-pallets/authorities-noting/src/benchmarks.rs index efe9c50..54b0392 100644 --- a/container-chain-pallets/authorities-noting/src/benchmarks.rs +++ b/container-chain-pallets/authorities-noting/src/benchmarks.rs @@ -69,8 +69,8 @@ mod test_sproof { benchmarks! { set_latest_authorities_data { // TODO: this could measure the proof size - let sproof_builder_relay = test_sproof::ParaHeaderSproofBuilder::default(); - let sproof_builder_orchestrator = test_sproof::AuthorityAssignmentSproofBuilder::default(); + let sproof_builder_relay = test_sproof::ParaHeaderSproofBuilder; + let sproof_builder_orchestrator = test_sproof::AuthorityAssignmentSproofBuilder; let (relay_root, relay_proof) = sproof_builder_relay.into_state_root_and_proof(); let (orchestrator_root, orchestrator_proof) = sproof_builder_orchestrator.into_state_root_and_proof(); diff --git a/container-chain-pallets/authorities-noting/src/tests.rs b/container-chain-pallets/authorities-noting/src/tests.rs index caac4ee..4265365 100644 --- a/container-chain-pallets/authorities-noting/src/tests.rs +++ b/container-chain-pallets/authorities-noting/src/tests.rs @@ -41,7 +41,7 @@ fn genesis_config_orchestrator_para_id() { fn genesis_config_orchestrator_para_id_storage_update() { new_test_ext().execute_with(|| { let new_para_id = ParaId::new(2000); - OrchestratorParaId::::put(&new_para_id); + OrchestratorParaId::::put(new_para_id); assert_eq!(OrchestratorParaId::::get(), new_para_id); }); } diff --git a/container-chain-primitives/authorities-noting-inherent/src/tests.rs b/container-chain-primitives/authorities-noting-inherent/src/tests.rs index d009e5d..1135cb1 100644 --- a/container-chain-primitives/authorities-noting-inherent/src/tests.rs +++ b/container-chain-primitives/authorities-noting-inherent/src/tests.rs @@ -26,7 +26,7 @@ use { InboundDownwardMessage, InboundHrmpMessage, ParaId, PersistedValidationData, }, cumulus_relay_chain_interface::{PHash, PHeader, RelayChainInterface, RelayChainResult}, - dc_orchestrator_chain_interface::{OrchestratorChainInterface, OrchestratorChainResult}, + dc_orchestrator_chain_interface::{OrchestratorChainInterface, OrchestratorChainResult, Slot}, dp_core::{well_known_keys, Header as OrchestratorHeader}, futures::Stream, polkadot_overseer::Handle, @@ -84,6 +84,8 @@ impl DummyRelayChainInterface { #[async_trait] impl OrchestratorChainInterface for DummyOrchestratorChainInterface { + type AuthorityId = (); + fn overseer_handle(&self) -> OrchestratorChainResult { unimplemented!("Not needed for test") } @@ -94,7 +96,7 @@ impl OrchestratorChainInterface for DummyOrchestratorChainInterface { key: &[u8], ) -> OrchestratorChainResult> { self.orchestrator_client - .storage(hash.into(), &StorageKey(key.to_vec())) + .storage(hash, &StorageKey(key.to_vec())) .map(|a| a.map(|b| b.0)) .map_err(|e| e.into()) } @@ -128,6 +130,22 @@ impl OrchestratorChainInterface for DummyOrchestratorChainInterface { ) -> OrchestratorChainResult + Send>>> { unimplemented!("Not needed for test") } + + async fn authorities( + &self, + _orchestrator_parent: PHash, + _para_id: ParaId, + ) -> OrchestratorChainResult>> { + unimplemented!("Not needed for test") + } + + async fn min_slot_freq( + &self, + _orchestrator_parent: PHash, + _para_id: ParaId, + ) -> OrchestratorChainResult> { + unimplemented!("Not needed for test") + } } #[async_trait] @@ -218,7 +236,7 @@ impl RelayChainInterface for DummyRelayChainInterface { ) -> RelayChainResult> { Ok(self .relay_client - .storage(hash.into(), &StorageKey(key.to_vec())) + .storage(hash, &StorageKey(key.to_vec())) .map(|a| a.map(|b| b.0)) .unwrap()) } diff --git a/pallets/xcm-executor-utils/src/benchmarks.rs b/pallets/xcm-executor-utils/src/benchmarks.rs index 42ce228..e53948f 100644 --- a/pallets/xcm-executor-utils/src/benchmarks.rs +++ b/pallets/xcm-executor-utils/src/benchmarks.rs @@ -115,5 +115,9 @@ mod benchmarks { Ok(()) } - impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test); + impl_benchmark_test_suite!( + Pallet, + crate::mock::mock_never::new_test_ext(), + crate::mock::mock_never::TestNever + ); } diff --git a/pallets/xcm-executor-utils/src/filters.rs b/pallets/xcm-executor-utils/src/filters.rs index d4f19fd..bba3b0f 100644 --- a/pallets/xcm-executor-utils/src/filters.rs +++ b/pallets/xcm-executor-utils/src/filters.rs @@ -174,15 +174,12 @@ mod test { fun: Fungible(1_000), }; - assert_eq!( - apply_policy::( - &grandparent_asset, - &parent_location, - None, - ::ReserveDefaultTrustPolicy::get(), - ), - false - ); + assert!(!apply_policy::( + &grandparent_asset, + &parent_location, + None, + ::ReserveDefaultTrustPolicy::get(), + )); } #[test] @@ -193,15 +190,12 @@ mod test { fun: Fungible(1_000), }; - assert_eq!( - apply_policy::( - &parent_asset, - &parent_location, - None, - ::ReserveDefaultTrustPolicy::get(), - ), - false - ); + assert!(!apply_policy::( + &parent_asset, + &parent_location, + None, + ::ReserveDefaultTrustPolicy::get(), + )); } #[test] @@ -260,15 +254,12 @@ mod test { DefaultTrustPolicy::AllNative, )); - assert_eq!( - apply_policy::( - &grandparent_asset, - &parent_location, - origin_policy, - default_policy - ), - false - ); + assert!(!apply_policy::( + &grandparent_asset, + &parent_location, + origin_policy, + default_policy + )); } #[test] @@ -314,14 +305,11 @@ mod test { )); // parent_asset should be rejected - assert_eq!( - apply_policy::( - &parent_asset, - &parent_location, - origin_policy, - default_policy - ), - false - ); + assert!(!apply_policy::( + &parent_asset, + &parent_location, + origin_policy, + default_policy + )); } } diff --git a/pallets/xcm-executor-utils/src/migrations.rs b/pallets/xcm-executor-utils/src/migrations.rs index 7a7ce69..3e76f7e 100644 --- a/pallets/xcm-executor-utils/src/migrations.rs +++ b/pallets/xcm-executor-utils/src/migrations.rs @@ -14,18 +14,20 @@ // You should have received a copy of the GNU General Public License // along with Tanssi. If not, see . -use super::*; -use frame_support::migration::storage_key_iter; -use frame_support::traits::OnRuntimeUpgrade; -use sp_std::vec::Vec; +use { + super::*, + frame_support::{migration::storage_key_iter, traits::OnRuntimeUpgrade}, + sp_std::vec::Vec, +}; /// The in-code storage version. pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); pub mod v1 { - use frame_support::pallet_prelude::*; - use sp_std::vec::Vec; - use staging_xcm::latest::AssetId; - use staging_xcm::v3::AssetId as OldAssetId; + use { + frame_support::pallet_prelude::*, + sp_std::vec::Vec, + staging_xcm::{latest::AssetId, v3::AssetId as OldAssetId}, + }; use crate::TrustPolicy; @@ -45,16 +47,16 @@ pub mod v1 { AllowedAssets(BoundedVec), } - impl> Into> for OldTrustPolicy { - fn into(self) -> TrustPolicy { - match self { + impl> From> for TrustPolicy { + fn from(val: OldTrustPolicy) -> Self { + match val { OldTrustPolicy::DefaultTrustPolicy(default_policy) => { TrustPolicy::DefaultTrustPolicy(default_policy) } OldTrustPolicy::AllowedAssets(old_assets) => { let new_assets: Vec = old_assets .iter() - .filter_map(|old_asset| AssetId::try_from(old_asset.clone()).ok()) + .filter_map(|old_asset| AssetId::try_from(*old_asset).ok()) .collect(); TrustPolicy::AllowedAssets( new_assets diff --git a/pallets/xcm-executor-utils/src/tests.rs b/pallets/xcm-executor-utils/src/tests.rs index 6da0da0..e420068 100644 --- a/pallets/xcm-executor-utils/src/tests.rs +++ b/pallets/xcm-executor-utils/src/tests.rs @@ -105,10 +105,10 @@ fn reserve_policy_is_applied() { )); // Should reject parent_asset - assert_eq!( - IsReserveFilter::::contains(&parent_asset, &parent_location), - false - ); + assert!(!IsReserveFilter::::contains( + &parent_asset, + &parent_location + )); }); } @@ -143,21 +143,23 @@ fn teleport_policy_is_applied() { ),); // Should reject parent_asset - assert_eq!( - IsTeleportFilter::::contains(&parent_asset, &parent_location), - false - ); + assert!(!IsTeleportFilter::::contains( + &parent_asset, + &parent_location + )); }); } #[test] fn test_v1_migration() { new_test_ext().execute_with(|| { - use frame_support::storage::migration::put_storage_value; - use frame_support::traits::OnRuntimeUpgrade; - use frame_support::StorageHasher; - use frame_support::StoragePrefixedMap; - use staging_xcm::v3::{AssetId as OldAssetId, MultiLocation as OldLocation}; + use { + frame_support::{ + storage::migration::put_storage_value, traits::OnRuntimeUpgrade, StorageHasher, + StoragePrefixedMap, + }, + staging_xcm::v3::{AssetId as OldAssetId, MultiLocation as OldLocation}, + }; let pallet_prefix = ReservePolicy::::pallet_prefix(); let reserve_policy_storage_prefix = ReservePolicy::::storage_prefix(); let teleport_policy_storage_prefix = TeleportPolicy::::storage_prefix(); diff --git a/primitives/slot-duration-runtime-api/Cargo.toml b/primitives/slot-duration-runtime-api/Cargo.toml index acc5772..055eec9 100644 --- a/primitives/slot-duration-runtime-api/Cargo.toml +++ b/primitives/slot-duration-runtime-api/Cargo.toml @@ -17,6 +17,7 @@ sp-core = { workspace = true } sp-io = { workspace = true } sp-runtime = { workspace = true } sp-std = { workspace = true } + # Cumulus cumulus-primitives-core = { workspace = true }