From 5fee48357067c561b9d143aa7fedc89508991709 Mon Sep 17 00:00:00 2001 From: Ozgun Ozerk Date: Wed, 28 Aug 2024 14:07:34 +0300 Subject: [PATCH 1/9] design decisions --- crates/wrapper/.gitignore | 2 + crates/wrapper/Cargo.toml | 8 ++ crates/wrapper/src/lib.rs | 133 +++++++++++++++++++++++++ crates/wrapper/src/zombie_collator.rs | 18 ++++ crates/wrapper/src/zombie_node.rs | 18 ++++ crates/wrapper/src/zombie_parachain.rs | 0 crates/wrapper/src/zombienet.rs | 98 ++++++++++++++++++ 7 files changed, 277 insertions(+) create mode 100644 crates/wrapper/.gitignore create mode 100644 crates/wrapper/Cargo.toml create mode 100644 crates/wrapper/src/lib.rs create mode 100644 crates/wrapper/src/zombie_collator.rs create mode 100644 crates/wrapper/src/zombie_node.rs create mode 100644 crates/wrapper/src/zombie_parachain.rs create mode 100644 crates/wrapper/src/zombienet.rs diff --git a/crates/wrapper/.gitignore b/crates/wrapper/.gitignore new file mode 100644 index 000000000..4fffb2f89 --- /dev/null +++ b/crates/wrapper/.gitignore @@ -0,0 +1,2 @@ +/target +/Cargo.lock diff --git a/crates/wrapper/Cargo.toml b/crates/wrapper/Cargo.toml new file mode 100644 index 000000000..d1578e650 --- /dev/null +++ b/crates/wrapper/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "wrapper" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/crates/wrapper/src/lib.rs b/crates/wrapper/src/lib.rs new file mode 100644 index 000000000..27dc5122b --- /dev/null +++ b/crates/wrapper/src/lib.rs @@ -0,0 +1,133 @@ +pub struct ZombieNode { + name: String, + // other fields +} + +impl ZombieNode { + pub fn new(name: String) -> Self { + // name can't be empty! + ZombieNode { + name, // other fields with default initialization + } + } + + // consume and return a new object with the modifications + // to allow user to chain operations + pub fn otherMethod(mut self) -> Self {} +} + +// ------ + +pub struct ZombieCollator { + name: String, + // other fields +} + +impl ZombieNode { + pub fn new(name: String) -> Self { + // name can't be empty! + ZombieCollator { + name, // other fields with default initialization + } + } + + // consume and return a new object with the modifications + // to allow user to chain operations + pub fn otherMethod(mut self) -> Self {} +} + + +use std::marker::PhantomData; + +struct EmptyState; +struct IdOnly; +struct CollatorOnly; +struct IdAndCollator; + +// Object representing the final build object +pub struct ZombieParachain { + id: Option, + collators: Option>, + /* other fields */ +} + +// Trait for optional methods +// any methods related to other fields can go in here, +// these methods will be callable in any state of `ZombieParachain` +pub trait OptionalMethods { + pub fn c(self, c: String) -> Self; + pub fn d(self, d: String) -> Self; + pub fn e(self, e: String) -> Self; +} + +// Implement optional methods for all states +impl OptionalMethods for ZombieParachain { + pub fn c(mut self, c: String) -> Self { + self.c = Some(c); + self + } + + pub fn d(mut self, d: String) -> Self { + self.d = Some(d); + self + } + + pub fn e(mut self, e: String) -> Self { + self.e = Some(e); + self + } +} + +impl ZombieParachain { + pub fn new() -> Self { + Self { + id: None, + collators: None, + /* other fields */, + state: std::marker::PhantomData, + } + } + + pub fn set_id(self, id: u8) -> ZombieParachain { + ZombieParachain { + id: Some(id), + collators: self.collators, + /* other fields */ + } + } + + pub fn set_collators(self, collators: Vec) -> ZombieParachain { + // collators can't be empty! + + ZombieParachain { + id: self.id, + collators: Some(collators), + /* other fields */ + } + } +} + +impl ZombieParachain { + pub fn set_collators(self, collators: Vec) -> ZombieParachain { + // collators can't be empty! + + ZombieParachain { + id: self.id, + collators: Some(collators), + /* other fields */ + } + } +} + +impl ZombieParachain { + pub fn set_id(self, id: u8) -> ZombieParachain { + ZombieParachain { + id: Some(id), + collators: self.collators, + /* other fields */ + } + } +} + +// `ZombieParachain` will be used by `Zombienet` struct. + diff --git a/crates/wrapper/src/zombie_collator.rs b/crates/wrapper/src/zombie_collator.rs new file mode 100644 index 000000000..b5b57e97f --- /dev/null +++ b/crates/wrapper/src/zombie_collator.rs @@ -0,0 +1,18 @@ +pub struct ZombieCollator { + name: String, + // other fields +} + +impl ZombieNode { + pub fn new(name: String) -> Self { + // name can't be empty! + + ZombieCollator { + name, // other fields with default initialization + } + } + + // consume and return a new object with the modifications + // to allow user to chain operations + pub fn otherMethod(mut self) -> Self {} +} diff --git a/crates/wrapper/src/zombie_node.rs b/crates/wrapper/src/zombie_node.rs new file mode 100644 index 000000000..c4e36a177 --- /dev/null +++ b/crates/wrapper/src/zombie_node.rs @@ -0,0 +1,18 @@ +pub struct ZombieNode { + name: String, + // other fields +} + +impl ZombieNode { + pub fn new(name: String) -> Self { + // name can't be empty! + + ZombieNode { + name, // other fields with default initialization + } + } + + // consume and return a new object with the modifications + // to allow user to chain operations + pub fn otherMethod(mut self) -> Self {} +} diff --git a/crates/wrapper/src/zombie_parachain.rs b/crates/wrapper/src/zombie_parachain.rs new file mode 100644 index 000000000..e69de29bb diff --git a/crates/wrapper/src/zombienet.rs b/crates/wrapper/src/zombienet.rs new file mode 100644 index 000000000..b2184d61b --- /dev/null +++ b/crates/wrapper/src/zombienet.rs @@ -0,0 +1,98 @@ +use std::marker::PhantomData; + +struct EmptyState; +struct NodeOnly; +struct ParachainOnly; +struct NodeAndParachain; + +// Object representing the final build object +pub struct Zombienet { + nodes: Option>, + parachain: Option>, + /* other fields */ +} + +// Trait for optional methods +// any methods related to other fields can go in here, +// these methods will be callable in any state of `ZombieParachain` +pub trait OptionalMethods { + pub fn c(self, c: String) -> Self; + pub fn d(self, d: String) -> Self; + pub fn e(self, e: String) -> Self; +} + +// Implement optional methods for all states +impl OptionalMethods for Zombienet { + pub fn c(mut self, c: String) -> Self { + self.c = Some(c); + self + } + + pub fn d(mut self, d: String) -> Self { + self.d = Some(d); + self + } + + pub fn e(mut self, e: String) -> Self { + self.e = Some(e); + self + } +} + +impl Zombienet { + pub fn new() -> Self { + Self { + nodes: None, + parachain: None, + /* other fields */, + state: std::marker::PhantomData, + } + } + + pub fn set_nodes(self, nodes: Vec) -> Zombienet { + // nodes can't be empty! + + Zombienet { + nodes: Some(nodes), + parachain: self.parachain, + /* other fields */ + } + } + + pub fn set_parachain(self, parachain: ZombieParachain) -> Zombienet { + Zombienet { + nodes: self.nodes, + parachain: Some(parachain), + /* other fields */ + } + } +} + +impl ZombieParachain { + pub fn set_parachain(self, collators: Vec) -> Zombienet { + Zombienet { + nodes: self.nodes, + parachain: Some(parachain), + /* other fields */ + } + } +} + +impl Zombienet { + pub fn set_nodes(self, nodes: Vec) -> Zombienet { + // nodes can't be empty! + + Zombienet { + nodes: Some(nodes), + parachain: self.parachain, + /* other fields */ + } + } +} + + +impl Zombienet { + // spawn() is available only after both `nodes` and `parachain` are set + pub fn spawn(self) -> Result, OrchestratorError> {} +} + From 4ff17920e8268b712697d8c2ebacc63a122fbcbb Mon Sep 17 00:00:00 2001 From: Ozgun Ozerk Date: Wed, 18 Sep 2024 16:28:52 +0300 Subject: [PATCH 2/9] improve devx --- Cargo.toml | 3 +- crates/wrapper/src/lib.rs | 137 +------------------------ crates/wrapper/src/zombie_collator.rs | 3 +- crates/wrapper/src/zombie_node.rs | 1 + crates/wrapper/src/zombie_parachain.rs | 21 ++++ crates/wrapper/src/zombienet.rs | 112 +++++--------------- 6 files changed, 53 insertions(+), 224 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e5470c8dc..9ba84d7a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,8 @@ members = [ "crates/provider", #"crates/test-runner", "crates/prom-metrics-parser", - "crates/file-server" + "crates/file-server", + "crates/wrapper", ] [workspace.package] diff --git a/crates/wrapper/src/lib.rs b/crates/wrapper/src/lib.rs index 27dc5122b..45b973328 100644 --- a/crates/wrapper/src/lib.rs +++ b/crates/wrapper/src/lib.rs @@ -1,133 +1,4 @@ -pub struct ZombieNode { - name: String, - // other fields -} - -impl ZombieNode { - pub fn new(name: String) -> Self { - // name can't be empty! - ZombieNode { - name, // other fields with default initialization - } - } - - // consume and return a new object with the modifications - // to allow user to chain operations - pub fn otherMethod(mut self) -> Self {} -} - -// ------ - -pub struct ZombieCollator { - name: String, - // other fields -} - -impl ZombieNode { - pub fn new(name: String) -> Self { - // name can't be empty! - ZombieCollator { - name, // other fields with default initialization - } - } - - // consume and return a new object with the modifications - // to allow user to chain operations - pub fn otherMethod(mut self) -> Self {} -} - - -use std::marker::PhantomData; - -struct EmptyState; -struct IdOnly; -struct CollatorOnly; -struct IdAndCollator; - -// Object representing the final build object -pub struct ZombieParachain { - id: Option, - collators: Option>, - /* other fields */ -} - -// Trait for optional methods -// any methods related to other fields can go in here, -// these methods will be callable in any state of `ZombieParachain` -pub trait OptionalMethods { - pub fn c(self, c: String) -> Self; - pub fn d(self, d: String) -> Self; - pub fn e(self, e: String) -> Self; -} - -// Implement optional methods for all states -impl OptionalMethods for ZombieParachain { - pub fn c(mut self, c: String) -> Self { - self.c = Some(c); - self - } - - pub fn d(mut self, d: String) -> Self { - self.d = Some(d); - self - } - - pub fn e(mut self, e: String) -> Self { - self.e = Some(e); - self - } -} - -impl ZombieParachain { - pub fn new() -> Self { - Self { - id: None, - collators: None, - /* other fields */, - state: std::marker::PhantomData, - } - } - - pub fn set_id(self, id: u8) -> ZombieParachain { - ZombieParachain { - id: Some(id), - collators: self.collators, - /* other fields */ - } - } - - pub fn set_collators(self, collators: Vec) -> ZombieParachain { - // collators can't be empty! - - ZombieParachain { - id: self.id, - collators: Some(collators), - /* other fields */ - } - } -} - -impl ZombieParachain { - pub fn set_collators(self, collators: Vec) -> ZombieParachain { - // collators can't be empty! - - ZombieParachain { - id: self.id, - collators: Some(collators), - /* other fields */ - } - } -} - -impl ZombieParachain { - pub fn set_id(self, id: u8) -> ZombieParachain { - ZombieParachain { - id: Some(id), - collators: self.collators, - /* other fields */ - } - } -} - -// `ZombieParachain` will be used by `Zombienet` struct. - +pub mod zombie_collator; +pub mod zombie_node; +pub mod zombie_parachain; +pub mod zombienet; diff --git a/crates/wrapper/src/zombie_collator.rs b/crates/wrapper/src/zombie_collator.rs index b5b57e97f..b2cab1f28 100644 --- a/crates/wrapper/src/zombie_collator.rs +++ b/crates/wrapper/src/zombie_collator.rs @@ -3,7 +3,8 @@ pub struct ZombieCollator { // other fields } -impl ZombieNode { +impl ZombieCollator { + // takes the name of the collator as the parameter pub fn new(name: String) -> Self { // name can't be empty! diff --git a/crates/wrapper/src/zombie_node.rs b/crates/wrapper/src/zombie_node.rs index c4e36a177..6e123e4ff 100644 --- a/crates/wrapper/src/zombie_node.rs +++ b/crates/wrapper/src/zombie_node.rs @@ -4,6 +4,7 @@ pub struct ZombieNode { } impl ZombieNode { + // takes the name of the node as the parameter pub fn new(name: String) -> Self { // name can't be empty! diff --git a/crates/wrapper/src/zombie_parachain.rs b/crates/wrapper/src/zombie_parachain.rs index e69de29bb..b6a118521 100644 --- a/crates/wrapper/src/zombie_parachain.rs +++ b/crates/wrapper/src/zombie_parachain.rs @@ -0,0 +1,21 @@ +pub struct ZombieParachain { + id: u8, + collators: Vec, + // other fields +} + +impl ZombieParachain { + /// the first parameter is the ID of the parachain: `u8`, + /// the second parameter is the list of collators: `Vec` + pub fn new(id: u8, collators: Vec) -> Self { + Self { + id, + collators, + // other fields + } + } + + // consume and return a new object with the modifications + // to allow user to chain operations + pub fn otherMethod(mut self) -> Self {} +} diff --git a/crates/wrapper/src/zombienet.rs b/crates/wrapper/src/zombienet.rs index b2184d61b..b3c7e2a2a 100644 --- a/crates/wrapper/src/zombienet.rs +++ b/crates/wrapper/src/zombienet.rs @@ -1,98 +1,32 @@ -use std::marker::PhantomData; - -struct EmptyState; -struct NodeOnly; -struct ParachainOnly; -struct NodeAndParachain; - -// Object representing the final build object pub struct Zombienet { - nodes: Option>, - parachain: Option>, - /* other fields */ -} - -// Trait for optional methods -// any methods related to other fields can go in here, -// these methods will be callable in any state of `ZombieParachain` -pub trait OptionalMethods { - pub fn c(self, c: String) -> Self; - pub fn d(self, d: String) -> Self; - pub fn e(self, e: String) -> Self; -} - -// Implement optional methods for all states -impl OptionalMethods for Zombienet { - pub fn c(mut self, c: String) -> Self { - self.c = Some(c); - self - } - - pub fn d(mut self, d: String) -> Self { - self.d = Some(d); - self - } - - pub fn e(mut self, e: String) -> Self { - self.e = Some(e); - self - } + nodes: Vec, + parachain: ZombieParachain, + // other fields } -impl Zombienet { - pub fn new() -> Self { - Self { - nodes: None, - parachain: None, - /* other fields */, - state: std::marker::PhantomData, - } +impl Zombienet { + /// the first parameter is the list of nodes: `Vec` + /// the second parameter is the parachain: `ZombieParachain` + pub fn new(nodes: Vec, parachains: ZombieParachain) -> Self { + Self { + nodes, + parachain, + // other fields + } } - pub fn set_nodes(self, nodes: Vec) -> Zombienet { - // nodes can't be empty! + // consume and return a new object with the modifications + // to allow user to chain operations + pub fn otherMethod(mut self) -> Self {} - Zombienet { - nodes: Some(nodes), - parachain: self.parachain, - /* other fields */ - } - } - - pub fn set_parachain(self, parachain: ZombieParachain) -> Zombienet { - Zombienet { - nodes: self.nodes, - parachain: Some(parachain), - /* other fields */ - } - } -} + // spawn() is available only after both `nodes` and `parachain` are set + pub fn spawn(self) -> Result, OrchestratorError> { + if self.nodes.is_none() || self.parachain.is_none() { + return Err(OrchestratorError::InvalidConfig( + "`nodes` or `parachain` field is not set for the network.", + )); + } -impl ZombieParachain { - pub fn set_parachain(self, collators: Vec) -> Zombienet { - Zombienet { - nodes: self.nodes, - parachain: Some(parachain), - /* other fields */ - } + // rest of the body } } - -impl Zombienet { - pub fn set_nodes(self, nodes: Vec) -> Zombienet { - // nodes can't be empty! - - Zombienet { - nodes: Some(nodes), - parachain: self.parachain, - /* other fields */ - } - } -} - - -impl Zombienet { - // spawn() is available only after both `nodes` and `parachain` are set - pub fn spawn(self) -> Result, OrchestratorError> {} -} - From c1fdeabee32e600923dec4e4d0219f6fcd32d994 Mon Sep 17 00:00:00 2001 From: Ozgun Ozerk Date: Fri, 20 Sep 2024 21:55:31 +0300 Subject: [PATCH 3/9] wrapper approach --- crates/configuration/src/lib.rs | 11 +++-- crates/wrapper/Cargo.toml | 1 + crates/wrapper/src/zombie_node.rs | 8 +++- crates/wrapper/src/zombienet.rs | 72 +++++++++++++++++++++++++------ 4 files changed, 75 insertions(+), 17 deletions(-) diff --git a/crates/configuration/src/lib.rs b/crates/configuration/src/lib.rs index 799ab1c6b..3e3dd9af9 100644 --- a/crates/configuration/src/lib.rs +++ b/crates/configuration/src/lib.rs @@ -87,11 +87,16 @@ pub mod shared; mod utils; pub use global_settings::{GlobalSettings, GlobalSettingsBuilder}; -pub use hrmp_channel::{HrmpChannelConfig, HrmpChannelConfigBuilder}; -pub use network::{NetworkConfig, NetworkConfigBuilder}; +pub use hrmp_channel::{ + HrmpChannelConfig, HrmpChannelConfigBuilder, Initial as HrmpInitialState, WithRecipient, +}; +pub use network::{NetworkConfig, NetworkConfigBuilder, WithRelaychain}; pub use parachain::{ states as para_states, ParachainConfig, ParachainConfigBuilder, RegistrationStrategy, }; -pub use relaychain::{RelaychainConfig, RelaychainConfigBuilder}; +pub use relaychain::{ + Initial as RelaychainInitialState, RelaychainConfig, RelaychainConfigBuilder, + WithAtLeastOneNode, +}; // re-export shared pub use shared::{node::NodeConfig, types}; diff --git a/crates/wrapper/Cargo.toml b/crates/wrapper/Cargo.toml index d1578e650..59d71ccd3 100644 --- a/crates/wrapper/Cargo.toml +++ b/crates/wrapper/Cargo.toml @@ -6,3 +6,4 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +configuration = { workspace = true } diff --git a/crates/wrapper/src/zombie_node.rs b/crates/wrapper/src/zombie_node.rs index 6e123e4ff..2e2080fef 100644 --- a/crates/wrapper/src/zombie_node.rs +++ b/crates/wrapper/src/zombie_node.rs @@ -13,7 +13,13 @@ impl ZombieNode { } } + pub fn name(&self) -> &str { + self.name.as_str() + } + // consume and return a new object with the modifications // to allow user to chain operations - pub fn otherMethod(mut self) -> Self {} + pub fn otherMethod(mut self) -> Self { + self + } } diff --git a/crates/wrapper/src/zombienet.rs b/crates/wrapper/src/zombienet.rs index b3c7e2a2a..953af0ef5 100644 --- a/crates/wrapper/src/zombienet.rs +++ b/crates/wrapper/src/zombienet.rs @@ -1,23 +1,69 @@ -pub struct Zombienet { - nodes: Vec, +use configuration::{ + GlobalSettingsBuilder, HrmpChannelConfigBuilder, HrmpInitialState, NetworkConfigBuilder, + RelaychainConfigBuilder, RelaychainInitialState, WithAtLeastOneNode, WithRecipient, + WithRelaychain, +}; + +use crate::{zombie_node::ZombieNode, zombie_parachain::ZombieParachain}; + +pub struct ZombienetBuilder { parachain: ZombieParachain, - // other fields + network: NetworkConfigBuilder, } -impl Zombienet { +impl ZombienetBuilder { /// the first parameter is the list of nodes: `Vec` /// the second parameter is the parachain: `ZombieParachain` - pub fn new(nodes: Vec, parachains: ZombieParachain) -> Self { - Self { - nodes, - parachain, - // other fields - } + /// relaychain and node settings are set to default, + /// use `new_custom` to set a custom relaychain and custom node settings. + pub fn new(nodes: Vec, parachain: ZombieParachain) -> Self { + let network = NetworkConfigBuilder::new().with_relaychain(|relaychain| { + let mut relayhcain_with_node = relaychain + .with_chain("rococo-local") + .with_node(|node| node.with_name(nodes.first().unwrap().name())); + + for node in nodes.iter().skip(1) { + relayhcain_with_node = relayhcain_with_node + .with_node(|node_builder| node_builder.with_name(node.name())); + } + relayhcain_with_node + }); + + Self { network, parachain } } - // consume and return a new object with the modifications - // to allow user to chain operations - pub fn otherMethod(mut self) -> Self {} + /// allows you to fully configure the relaychain, + /// the closure `f` you provide has to initialize nodes as well + /// use `new` if you want to go with the default relaychain + pub fn new_custom( + parachain: ZombieParachain, + f: impl FnOnce( + RelaychainConfigBuilder, + ) -> RelaychainConfigBuilder, + ) -> Self { + let network = NetworkConfigBuilder::new().with_relaychain(f); + + Self { network, parachain } + } + + /// optional method for setting hrmp channel, hrmp is not set by default + pub fn set_hrmp_channel( + mut self, + f: impl FnOnce( + HrmpChannelConfigBuilder, + ) -> HrmpChannelConfigBuilder, + ) -> Self { + self.network = self.network.with_hrmp_channel(f); + self + } + + pub fn set_global_settings( + mut self, + f: impl FnOnce(GlobalSettingsBuilder) -> GlobalSettingsBuilder, + ) -> Self { + self.network = self.network.with_global_settings(f); + self + } // spawn() is available only after both `nodes` and `parachain` are set pub fn spawn(self) -> Result, OrchestratorError> { From b6094370dd825c93a7e343d4ecdb9ba183e32fb7 Mon Sep 17 00:00:00 2001 From: Ozgun Ozerk Date: Sat, 21 Sep 2024 00:10:11 +0300 Subject: [PATCH 4/9] additional methods instead of a new crate --- Cargo.toml | 1 - crates/configuration/src/network.rs | 47 ++++++++++++++++ crates/wrapper/.gitignore | 2 - crates/wrapper/Cargo.toml | 9 --- crates/wrapper/src/lib.rs | 4 -- crates/wrapper/src/zombie_collator.rs | 19 ------- crates/wrapper/src/zombie_node.rs | 25 --------- crates/wrapper/src/zombie_parachain.rs | 21 ------- crates/wrapper/src/zombienet.rs | 78 -------------------------- 9 files changed, 47 insertions(+), 159 deletions(-) delete mode 100644 crates/wrapper/.gitignore delete mode 100644 crates/wrapper/Cargo.toml delete mode 100644 crates/wrapper/src/lib.rs delete mode 100644 crates/wrapper/src/zombie_collator.rs delete mode 100644 crates/wrapper/src/zombie_node.rs delete mode 100644 crates/wrapper/src/zombie_parachain.rs delete mode 100644 crates/wrapper/src/zombienet.rs diff --git a/Cargo.toml b/Cargo.toml index 9ba84d7a4..0e052f958 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,6 @@ members = [ #"crates/test-runner", "crates/prom-metrics-parser", "crates/file-server", - "crates/wrapper", ] [workspace.package] diff --git a/crates/configuration/src/network.rs b/crates/configuration/src/network.rs index 1b133018b..fb439487b 100644 --- a/crates/configuration/src/network.rs +++ b/crates/configuration/src/network.rs @@ -324,6 +324,27 @@ impl NetworkConfigBuilder { Self::default() } + /// uses the default options for both the relay chain and the nodes + /// the only required fields are the name of the nodes, + /// and the name of the relay chain ("rococo-local", "polkadot", etc.) + pub fn new_with_defaults( + node_names: Vec, + relay_name: &str, + ) -> NetworkConfigBuilder { + // TODO: check node_names is not empty + NetworkConfigBuilder::new().with_relaychain(|relaychain| { + let mut relaychain_with_node = relaychain + .with_chain(relay_name) + .with_node(|node| node.with_name(node_names.first().unwrap())); + + for node_name in node_names.iter().skip(1) { + relaychain_with_node = relaychain_with_node + .with_node(|node_builder| node_builder.with_name(node_name)); + } + relaychain_with_node + }) + } + /// Set the relay chain using a nested [`RelaychainConfigBuilder`]. pub fn with_relaychain( self, @@ -399,6 +420,32 @@ impl NetworkConfigBuilder { } } + /// uses default settings for setting for: + /// - the parachain, + /// - the global settings + /// - the hrmp channels + /// the only required parameters are the names of the collators, + /// and the id of the parachain + pub fn with_parachain_defaults(self, collators: Vec, id: u32) -> Self { + // TODO: check collators is not empty -> add it into validation_context errors + self.with_parachain(|parachain| { + let mut parachain_config = parachain.with_id(id).with_collator(|collator| { + collator + .with_name(collators.first().unwrap()) + .validator(true) + }); + + for collator_name in collators.iter().skip(1) { + parachain_config = parachain_config + .with_collator(|collator| collator.with_name(collator_name).validator(true)); + } + parachain_config + }) + + // TODO: if need to set global settings and hrmp channels + // we can also do in here + } + /// Add an HRMP channel using a nested [`HrmpChannelConfigBuilder`]. pub fn with_hrmp_channel( self, diff --git a/crates/wrapper/.gitignore b/crates/wrapper/.gitignore deleted file mode 100644 index 4fffb2f89..000000000 --- a/crates/wrapper/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -/target -/Cargo.lock diff --git a/crates/wrapper/Cargo.toml b/crates/wrapper/Cargo.toml deleted file mode 100644 index 59d71ccd3..000000000 --- a/crates/wrapper/Cargo.toml +++ /dev/null @@ -1,9 +0,0 @@ -[package] -name = "wrapper" -version = "0.1.0" -edition = "2021" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -[dependencies] -configuration = { workspace = true } diff --git a/crates/wrapper/src/lib.rs b/crates/wrapper/src/lib.rs deleted file mode 100644 index 45b973328..000000000 --- a/crates/wrapper/src/lib.rs +++ /dev/null @@ -1,4 +0,0 @@ -pub mod zombie_collator; -pub mod zombie_node; -pub mod zombie_parachain; -pub mod zombienet; diff --git a/crates/wrapper/src/zombie_collator.rs b/crates/wrapper/src/zombie_collator.rs deleted file mode 100644 index b2cab1f28..000000000 --- a/crates/wrapper/src/zombie_collator.rs +++ /dev/null @@ -1,19 +0,0 @@ -pub struct ZombieCollator { - name: String, - // other fields -} - -impl ZombieCollator { - // takes the name of the collator as the parameter - pub fn new(name: String) -> Self { - // name can't be empty! - - ZombieCollator { - name, // other fields with default initialization - } - } - - // consume and return a new object with the modifications - // to allow user to chain operations - pub fn otherMethod(mut self) -> Self {} -} diff --git a/crates/wrapper/src/zombie_node.rs b/crates/wrapper/src/zombie_node.rs deleted file mode 100644 index 2e2080fef..000000000 --- a/crates/wrapper/src/zombie_node.rs +++ /dev/null @@ -1,25 +0,0 @@ -pub struct ZombieNode { - name: String, - // other fields -} - -impl ZombieNode { - // takes the name of the node as the parameter - pub fn new(name: String) -> Self { - // name can't be empty! - - ZombieNode { - name, // other fields with default initialization - } - } - - pub fn name(&self) -> &str { - self.name.as_str() - } - - // consume and return a new object with the modifications - // to allow user to chain operations - pub fn otherMethod(mut self) -> Self { - self - } -} diff --git a/crates/wrapper/src/zombie_parachain.rs b/crates/wrapper/src/zombie_parachain.rs deleted file mode 100644 index b6a118521..000000000 --- a/crates/wrapper/src/zombie_parachain.rs +++ /dev/null @@ -1,21 +0,0 @@ -pub struct ZombieParachain { - id: u8, - collators: Vec, - // other fields -} - -impl ZombieParachain { - /// the first parameter is the ID of the parachain: `u8`, - /// the second parameter is the list of collators: `Vec` - pub fn new(id: u8, collators: Vec) -> Self { - Self { - id, - collators, - // other fields - } - } - - // consume and return a new object with the modifications - // to allow user to chain operations - pub fn otherMethod(mut self) -> Self {} -} diff --git a/crates/wrapper/src/zombienet.rs b/crates/wrapper/src/zombienet.rs deleted file mode 100644 index 953af0ef5..000000000 --- a/crates/wrapper/src/zombienet.rs +++ /dev/null @@ -1,78 +0,0 @@ -use configuration::{ - GlobalSettingsBuilder, HrmpChannelConfigBuilder, HrmpInitialState, NetworkConfigBuilder, - RelaychainConfigBuilder, RelaychainInitialState, WithAtLeastOneNode, WithRecipient, - WithRelaychain, -}; - -use crate::{zombie_node::ZombieNode, zombie_parachain::ZombieParachain}; - -pub struct ZombienetBuilder { - parachain: ZombieParachain, - network: NetworkConfigBuilder, -} - -impl ZombienetBuilder { - /// the first parameter is the list of nodes: `Vec` - /// the second parameter is the parachain: `ZombieParachain` - /// relaychain and node settings are set to default, - /// use `new_custom` to set a custom relaychain and custom node settings. - pub fn new(nodes: Vec, parachain: ZombieParachain) -> Self { - let network = NetworkConfigBuilder::new().with_relaychain(|relaychain| { - let mut relayhcain_with_node = relaychain - .with_chain("rococo-local") - .with_node(|node| node.with_name(nodes.first().unwrap().name())); - - for node in nodes.iter().skip(1) { - relayhcain_with_node = relayhcain_with_node - .with_node(|node_builder| node_builder.with_name(node.name())); - } - relayhcain_with_node - }); - - Self { network, parachain } - } - - /// allows you to fully configure the relaychain, - /// the closure `f` you provide has to initialize nodes as well - /// use `new` if you want to go with the default relaychain - pub fn new_custom( - parachain: ZombieParachain, - f: impl FnOnce( - RelaychainConfigBuilder, - ) -> RelaychainConfigBuilder, - ) -> Self { - let network = NetworkConfigBuilder::new().with_relaychain(f); - - Self { network, parachain } - } - - /// optional method for setting hrmp channel, hrmp is not set by default - pub fn set_hrmp_channel( - mut self, - f: impl FnOnce( - HrmpChannelConfigBuilder, - ) -> HrmpChannelConfigBuilder, - ) -> Self { - self.network = self.network.with_hrmp_channel(f); - self - } - - pub fn set_global_settings( - mut self, - f: impl FnOnce(GlobalSettingsBuilder) -> GlobalSettingsBuilder, - ) -> Self { - self.network = self.network.with_global_settings(f); - self - } - - // spawn() is available only after both `nodes` and `parachain` are set - pub fn spawn(self) -> Result, OrchestratorError> { - if self.nodes.is_none() || self.parachain.is_none() { - return Err(OrchestratorError::InvalidConfig( - "`nodes` or `parachain` field is not set for the network.", - )); - } - - // rest of the body - } -} From e3396224b9daa3561d03ea81ac5ffc8eadae6cc8 Mon Sep 17 00:00:00 2001 From: Ozgun Ozerk Date: Sat, 21 Sep 2024 13:29:39 +0300 Subject: [PATCH 5/9] tests --- crates/configuration/src/network.rs | 135 ++++++++++++++++++++++++++-- 1 file changed, 127 insertions(+), 8 deletions(-) diff --git a/crates/configuration/src/network.rs b/crates/configuration/src/network.rs index fb439487b..a47fcbc25 100644 --- a/crates/configuration/src/network.rs +++ b/crates/configuration/src/network.rs @@ -13,7 +13,8 @@ use crate::{ parachain::{self, ParachainConfig, ParachainConfigBuilder}, relaychain::{self, RelaychainConfig, RelaychainConfigBuilder}, shared::{ - helpers::merge_errors_vecs, + errors::ConfigError, + helpers::{merge_errors, merge_errors_vecs}, macros::states, node::NodeConfig, types::{Arg, AssetLocation, Chain, Command, Image, ValidationContext}, @@ -331,18 +332,33 @@ impl NetworkConfigBuilder { node_names: Vec, relay_name: &str, ) -> NetworkConfigBuilder { - // TODO: check node_names is not empty - NetworkConfigBuilder::new().with_relaychain(|relaychain| { + let mut errors: Vec = vec![]; + + if node_names.is_empty() { + errors.push(ConfigError::Relaychain(anyhow!("node names list can't be empty")).into()) + } + + if relay_name.is_empty() { + errors.push(ConfigError::Relaychain(anyhow!("relaychain name can't be empty")).into()); + } + + let network_config = NetworkConfigBuilder::new().with_relaychain(|relaychain| { let mut relaychain_with_node = relaychain .with_chain(relay_name) - .with_node(|node| node.with_name(node_names.first().unwrap())); + .with_node(|node| node.with_name(node_names.first().unwrap_or(&"".to_string()))); for node_name in node_names.iter().skip(1) { relaychain_with_node = relaychain_with_node .with_node(|node_builder| node_builder.with_name(node_name)); } relaychain_with_node - }) + }); + + Self::transition( + network_config.config, + network_config.validation_context, + errors, + ) } /// Set the relay chain using a nested [`RelaychainConfigBuilder`]. @@ -424,10 +440,21 @@ impl NetworkConfigBuilder { /// - the parachain, /// - the global settings /// - the hrmp channels - /// the only required parameters are the names of the collators, + /// the only required parameters are the names of the collators as a vector, /// and the id of the parachain - pub fn with_parachain_defaults(self, collators: Vec, id: u32) -> Self { - // TODO: check collators is not empty -> add it into validation_context errors + pub fn quick_setup(self, collators: Vec, id: u32) -> Self { + if collators.is_empty() { + return Self::transition( + self.config, + self.validation_context, + merge_errors( + self.errors, + ConfigError::Parachain(id, anyhow!("collator names list can't be empty")) + .into(), + ), + ); + } + self.with_parachain(|parachain| { let mut parachain_config = parachain.with_id(id).with_collator(|collator| { collator @@ -1484,4 +1511,96 @@ mod tests { }); }); } + + #[test] + fn new_with_defaults_works() { + let network_config = NetworkConfigBuilder::new_with_defaults( + vec!["alice".to_string(), "bob".to_string()], + "rococo-local", + ) + .build() + .unwrap(); + + // relaychain + assert_eq!(network_config.relaychain().chain().as_str(), "rococo-local"); + assert_eq!(network_config.relaychain().nodes().len(), 2); + let mut node_names = network_config.relaychain().nodes().into_iter(); + let node1 = node_names.next().unwrap().name(); + assert_eq!(node1, "alice"); + let node2 = node_names.next().unwrap().name(); + assert_eq!(node2, "bob"); + + // parachains + assert_eq!(network_config.parachains().len(), 0); + } + + #[test] + fn new_with_defaults_should_fail_with_empty_node_list() { + let errors = NetworkConfigBuilder::new_with_defaults(vec![], "rococo-local") + .build() + .unwrap_err(); + + assert_eq!( + errors.first().unwrap().to_string(), + "relaychain.node names list can't be empty" + ); + } + + #[test] + fn new_with_defaults_should_fail_with_empty_relay_name() { + let errors = NetworkConfigBuilder::new_with_defaults(vec!["alice".to_string()], "") + .build() + .unwrap_err(); + + assert_eq!( + errors.first().unwrap().to_string(), + "relaychain.relaychain name can't be empty" + ); + } + + #[test] + fn quick_setup_works() { + let network_config = NetworkConfigBuilder::new_with_defaults( + vec!["alice".to_string(), "bob".to_string()], + "rococo-local", + ) + .quick_setup(vec!["collator1".to_string(), "collator2".to_string()], 100) + .build() + .unwrap(); + + // relaychain + assert_eq!(network_config.relaychain().chain().as_str(), "rococo-local"); + assert_eq!(network_config.relaychain().nodes().len(), 2); + let mut node_names = network_config.relaychain().nodes().into_iter(); + let node1 = node_names.next().unwrap().name(); + assert_eq!(node1, "alice"); + let node2 = node_names.next().unwrap().name(); + assert_eq!(node2, "bob"); + + // parachains + assert_eq!(network_config.parachains().len(), 1); + let ¶chain1 = network_config.parachains().first().unwrap(); + assert_eq!(parachain1.id(), 100); + assert_eq!(parachain1.collators().len(), 2); + let mut collator_names = parachain1.collators().into_iter(); + let collator1 = collator_names.next().unwrap().name(); + assert_eq!(collator1, "collator1"); + let collator2 = collator_names.next().unwrap().name(); + assert_eq!(collator2, "collator2"); + + assert_eq!(parachain1.initial_balance(), 2_000_000_000_000); + } + + #[test] + fn quick_setup_should_fail_with_empty_collator_list() { + let errors = NetworkConfigBuilder::new_with_defaults(vec!["alice".to_string()], "polkadot") + .quick_setup(vec![], 1) + .build() + .unwrap_err(); + + assert_eq!( + errors.first().unwrap().to_string(), + "parachain[1].collator names list can't be empty" + ); + } } From c8e87cdfa0929d31770db929bbbf7ff9c2b5b6e1 Mon Sep 17 00:00:00 2001 From: Ozgun Ozerk Date: Sat, 21 Sep 2024 13:34:45 +0300 Subject: [PATCH 6/9] revert lib.rs changes --- crates/configuration/src/lib.rs | 11 +++-------- crates/configuration/src/network.rs | 1 + 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/configuration/src/lib.rs b/crates/configuration/src/lib.rs index 3e3dd9af9..799ab1c6b 100644 --- a/crates/configuration/src/lib.rs +++ b/crates/configuration/src/lib.rs @@ -87,16 +87,11 @@ pub mod shared; mod utils; pub use global_settings::{GlobalSettings, GlobalSettingsBuilder}; -pub use hrmp_channel::{ - HrmpChannelConfig, HrmpChannelConfigBuilder, Initial as HrmpInitialState, WithRecipient, -}; -pub use network::{NetworkConfig, NetworkConfigBuilder, WithRelaychain}; +pub use hrmp_channel::{HrmpChannelConfig, HrmpChannelConfigBuilder}; +pub use network::{NetworkConfig, NetworkConfigBuilder}; pub use parachain::{ states as para_states, ParachainConfig, ParachainConfigBuilder, RegistrationStrategy, }; -pub use relaychain::{ - Initial as RelaychainInitialState, RelaychainConfig, RelaychainConfigBuilder, - WithAtLeastOneNode, -}; +pub use relaychain::{RelaychainConfig, RelaychainConfigBuilder}; // re-export shared pub use shared::{node::NodeConfig, types}; diff --git a/crates/configuration/src/network.rs b/crates/configuration/src/network.rs index a47fcbc25..228f8e8c9 100644 --- a/crates/configuration/src/network.rs +++ b/crates/configuration/src/network.rs @@ -440,6 +440,7 @@ impl NetworkConfigBuilder { /// - the parachain, /// - the global settings /// - the hrmp channels + /// /// the only required parameters are the names of the collators as a vector, /// and the id of the parachain pub fn quick_setup(self, collators: Vec, id: u32) -> Self { From 8721d6483d32fe8c13774196fd1b9ca9c9e4df1c Mon Sep 17 00:00:00 2001 From: Ozgun Ozerk Date: Mon, 23 Sep 2024 12:01:06 +0300 Subject: [PATCH 7/9] better naming --- crates/configuration/src/network.rs | 36 ++++++++++++++++------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/crates/configuration/src/network.rs b/crates/configuration/src/network.rs index 228f8e8c9..6044d6b99 100644 --- a/crates/configuration/src/network.rs +++ b/crates/configuration/src/network.rs @@ -328,7 +328,7 @@ impl NetworkConfigBuilder { /// uses the default options for both the relay chain and the nodes /// the only required fields are the name of the nodes, /// and the name of the relay chain ("rococo-local", "polkadot", etc.) - pub fn new_with_defaults( + pub fn with_nodes_and_chain( node_names: Vec, relay_name: &str, ) -> NetworkConfigBuilder { @@ -443,7 +443,7 @@ impl NetworkConfigBuilder { /// /// the only required parameters are the names of the collators as a vector, /// and the id of the parachain - pub fn quick_setup(self, collators: Vec, id: u32) -> Self { + pub fn with_collators_and_parachain_id(self, collators: Vec, id: u32) -> Self { if collators.is_empty() { return Self::transition( self.config, @@ -1514,8 +1514,8 @@ mod tests { } #[test] - fn new_with_defaults_works() { - let network_config = NetworkConfigBuilder::new_with_defaults( + fn with_nodes_and_chain_works() { + let network_config = NetworkConfigBuilder::with_nodes_and_chain( vec!["alice".to_string(), "bob".to_string()], "rococo-local", ) @@ -1536,8 +1536,8 @@ mod tests { } #[test] - fn new_with_defaults_should_fail_with_empty_node_list() { - let errors = NetworkConfigBuilder::new_with_defaults(vec![], "rococo-local") + fn with_nodes_and_chain_should_fail_with_empty_node_list() { + let errors = NetworkConfigBuilder::with_nodes_and_chain(vec![], "rococo-local") .build() .unwrap_err(); @@ -1548,8 +1548,8 @@ mod tests { } #[test] - fn new_with_defaults_should_fail_with_empty_relay_name() { - let errors = NetworkConfigBuilder::new_with_defaults(vec!["alice".to_string()], "") + fn with_nodes_and_chain_should_fail_with_empty_relay_name() { + let errors = NetworkConfigBuilder::with_nodes_and_chain(vec!["alice".to_string()], "") .build() .unwrap_err(); @@ -1560,12 +1560,15 @@ mod tests { } #[test] - fn quick_setup_works() { - let network_config = NetworkConfigBuilder::new_with_defaults( + fn with_collators_and_parachain_id_works() { + let network_config = NetworkConfigBuilder::with_nodes_and_chain( vec!["alice".to_string(), "bob".to_string()], "rococo-local", ) - .quick_setup(vec!["collator1".to_string(), "collator2".to_string()], 100) + .with_collators_and_parachain_id( + vec!["collator1".to_string(), "collator2".to_string()], + 100, + ) .build() .unwrap(); @@ -1593,11 +1596,12 @@ mod tests { } #[test] - fn quick_setup_should_fail_with_empty_collator_list() { - let errors = NetworkConfigBuilder::new_with_defaults(vec!["alice".to_string()], "polkadot") - .quick_setup(vec![], 1) - .build() - .unwrap_err(); + fn with_collators_and_parachain_id_should_fail_with_empty_collator_list() { + let errors = + NetworkConfigBuilder::with_nodes_and_chain(vec!["alice".to_string()], "polkadot") + .with_collators_and_parachain_id(vec![], 1) + .build() + .unwrap_err(); assert_eq!( errors.first().unwrap().to_string(), From c7ca8ca45bd7513b63869fb24708fb8f76cd8825 Mon Sep 17 00:00:00 2001 From: Ozgun Ozerk Date: Thu, 26 Sep 2024 09:25:48 +0300 Subject: [PATCH 8/9] suggestions addressed --- crates/configuration/src/network.rs | 104 +++++++++++++++++++++------- 1 file changed, 78 insertions(+), 26 deletions(-) diff --git a/crates/configuration/src/network.rs b/crates/configuration/src/network.rs index 6044d6b99..86a8d9a28 100644 --- a/crates/configuration/src/network.rs +++ b/crates/configuration/src/network.rs @@ -328,9 +328,9 @@ impl NetworkConfigBuilder { /// uses the default options for both the relay chain and the nodes /// the only required fields are the name of the nodes, /// and the name of the relay chain ("rococo-local", "polkadot", etc.) - pub fn with_nodes_and_chain( - node_names: Vec, + pub fn with_chain_and_nodes( relay_name: &str, + node_names: Vec, ) -> NetworkConfigBuilder { let mut errors: Vec = vec![]; @@ -338,14 +338,20 @@ impl NetworkConfigBuilder { errors.push(ConfigError::Relaychain(anyhow!("node names list can't be empty")).into()) } + for name in node_names.iter() { + if name.is_empty() { + errors.push(ConfigError::Relaychain(anyhow!("node name can't be empty")).into()); + } + } + if relay_name.is_empty() { errors.push(ConfigError::Relaychain(anyhow!("relaychain name can't be empty")).into()); } let network_config = NetworkConfigBuilder::new().with_relaychain(|relaychain| { - let mut relaychain_with_node = relaychain - .with_chain(relay_name) - .with_node(|node| node.with_name(node_names.first().unwrap_or(&"".to_string()))); + let mut relaychain_with_node = relaychain.with_chain(relay_name).with_node(|node| { + node.with_name(node_names.first().unwrap_or(&"node1".to_string())) + }); // TODO: in order to not panic this, we provide a dummy value. However, returning a Result may be a better choice for this constructor for node_name in node_names.iter().skip(1) { relaychain_with_node = relaychain_with_node @@ -443,8 +449,8 @@ impl NetworkConfigBuilder { /// /// the only required parameters are the names of the collators as a vector, /// and the id of the parachain - pub fn with_collators_and_parachain_id(self, collators: Vec, id: u32) -> Self { - if collators.is_empty() { + pub fn with_parachain_id_and_collators(self, id: u32, collator_names: Vec) -> Self { + if collator_names.is_empty() { return Self::transition( self.config, self.validation_context, @@ -456,14 +462,31 @@ impl NetworkConfigBuilder { ); } + for name in collator_names.iter() { + if name.is_empty() { + return Self::transition( + self.config, + self.validation_context, + merge_errors( + self.errors, + ConfigError::Parachain(id, anyhow!("collator name can't be empty")).into(), + ), + ); + } + } + self.with_parachain(|parachain| { let mut parachain_config = parachain.with_id(id).with_collator(|collator| { collator - .with_name(collators.first().unwrap()) + .with_name( + collator_names + .first() + .expect("collator names list is not empty"), + ) .validator(true) }); - for collator_name in collators.iter().skip(1) { + for collator_name in collator_names.iter().skip(1) { parachain_config = parachain_config .with_collator(|collator| collator.with_name(collator_name).validator(true)); } @@ -1514,10 +1537,10 @@ mod tests { } #[test] - fn with_nodes_and_chain_works() { - let network_config = NetworkConfigBuilder::with_nodes_and_chain( - vec!["alice".to_string(), "bob".to_string()], + fn with_chain_and_nodes_works() { + let network_config = NetworkConfigBuilder::with_chain_and_nodes( "rococo-local", + vec!["alice".to_string(), "bob".to_string()], ) .build() .unwrap(); @@ -1536,38 +1559,53 @@ mod tests { } #[test] - fn with_nodes_and_chain_should_fail_with_empty_node_list() { - let errors = NetworkConfigBuilder::with_nodes_and_chain(vec![], "rococo-local") + fn with_chain_and_nodes_should_fail_with_empty_relay_name() { + let errors = NetworkConfigBuilder::with_chain_and_nodes("", vec!["alice".to_string()]) .build() .unwrap_err(); assert_eq!( errors.first().unwrap().to_string(), - "relaychain.node names list can't be empty" + "relaychain.relaychain name can't be empty" ); } #[test] - fn with_nodes_and_chain_should_fail_with_empty_relay_name() { - let errors = NetworkConfigBuilder::with_nodes_and_chain(vec!["alice".to_string()], "") + fn with_chain_and_nodes_should_fail_with_empty_node_list() { + let errors = NetworkConfigBuilder::with_chain_and_nodes("rococo-local", vec![]) .build() .unwrap_err(); assert_eq!( errors.first().unwrap().to_string(), - "relaychain.relaychain name can't be empty" + "relaychain.node names list can't be empty" ); } #[test] - fn with_collators_and_parachain_id_works() { - let network_config = NetworkConfigBuilder::with_nodes_and_chain( - vec!["alice".to_string(), "bob".to_string()], + fn with_chain_and_nodes_should_fail_with_empty_node_name() { + let errors = NetworkConfigBuilder::with_chain_and_nodes( "rococo-local", + vec!["alice".to_string(), "".to_string()], ) - .with_collators_and_parachain_id( - vec!["collator1".to_string(), "collator2".to_string()], + .build() + .unwrap_err(); + + assert_eq!( + errors.first().unwrap().to_string(), + "relaychain.node name can't be empty" + ); + } + + #[test] + fn with_parachain_id_and_collators_works() { + let network_config = NetworkConfigBuilder::with_chain_and_nodes( + "rococo-local", + vec!["alice".to_string(), "bob".to_string()], + ) + .with_parachain_id_and_collators( 100, + vec!["collator1".to_string(), "collator2".to_string()], ) .build() .unwrap(); @@ -1596,10 +1634,10 @@ mod tests { } #[test] - fn with_collators_and_parachain_id_should_fail_with_empty_collator_list() { + fn with_parachain_id_and_collators_should_fail_with_empty_collator_list() { let errors = - NetworkConfigBuilder::with_nodes_and_chain(vec!["alice".to_string()], "polkadot") - .with_collators_and_parachain_id(vec![], 1) + NetworkConfigBuilder::with_chain_and_nodes("polkadot", vec!["alice".to_string()]) + .with_parachain_id_and_collators(1, vec![]) .build() .unwrap_err(); @@ -1608,4 +1646,18 @@ mod tests { "parachain[1].collator names list can't be empty" ); } + + #[test] + fn with_parachain_id_and_collators_should_fail_with_empty_collator_name() { + let errors = + NetworkConfigBuilder::with_chain_and_nodes("polkadot", vec!["alice".to_string()]) + .with_parachain_id_and_collators(1, vec!["collator1".to_string(), "".to_string()]) + .build() + .unwrap_err(); + + assert_eq!( + errors.first().unwrap().to_string(), + "parachain[1].collator name can't be empty" + ); + } } From 75594d40761eeaf73f9cf360c8c327cec543731e Mon Sep 17 00:00:00 2001 From: Ozgun Ozerk Date: Thu, 3 Oct 2024 13:59:59 +0300 Subject: [PATCH 9/9] suggestions --- crates/configuration/src/network.rs | 58 ++++++----------------------- 1 file changed, 12 insertions(+), 46 deletions(-) diff --git a/crates/configuration/src/network.rs b/crates/configuration/src/network.rs index 86a8d9a28..38622e28e 100644 --- a/crates/configuration/src/network.rs +++ b/crates/configuration/src/network.rs @@ -13,7 +13,7 @@ use crate::{ parachain::{self, ParachainConfig, ParachainConfigBuilder}, relaychain::{self, RelaychainConfig, RelaychainConfigBuilder}, shared::{ - errors::ConfigError, + errors::{ConfigError, ValidationError}, helpers::{merge_errors, merge_errors_vecs}, macros::states, node::NodeConfig, @@ -332,26 +332,10 @@ impl NetworkConfigBuilder { relay_name: &str, node_names: Vec, ) -> NetworkConfigBuilder { - let mut errors: Vec = vec![]; - - if node_names.is_empty() { - errors.push(ConfigError::Relaychain(anyhow!("node names list can't be empty")).into()) - } - - for name in node_names.iter() { - if name.is_empty() { - errors.push(ConfigError::Relaychain(anyhow!("node name can't be empty")).into()); - } - } - - if relay_name.is_empty() { - errors.push(ConfigError::Relaychain(anyhow!("relaychain name can't be empty")).into()); - } - let network_config = NetworkConfigBuilder::new().with_relaychain(|relaychain| { - let mut relaychain_with_node = relaychain.with_chain(relay_name).with_node(|node| { - node.with_name(node_names.first().unwrap_or(&"node1".to_string())) - }); // TODO: in order to not panic this, we provide a dummy value. However, returning a Result may be a better choice for this constructor + let mut relaychain_with_node = relaychain + .with_chain(relay_name) + .with_node(|node| node.with_name(node_names.first().unwrap_or(&"".to_string()))); for node_name in node_names.iter().skip(1) { relaychain_with_node = relaychain_with_node @@ -363,7 +347,7 @@ impl NetworkConfigBuilder { Self::transition( network_config.config, network_config.validation_context, - errors, + network_config.errors, ) } @@ -456,33 +440,15 @@ impl NetworkConfigBuilder { self.validation_context, merge_errors( self.errors, - ConfigError::Parachain(id, anyhow!("collator names list can't be empty")) - .into(), + ConfigError::Parachain(id, ValidationError::CantBeEmpty().into()).into(), ), ); } - for name in collator_names.iter() { - if name.is_empty() { - return Self::transition( - self.config, - self.validation_context, - merge_errors( - self.errors, - ConfigError::Parachain(id, anyhow!("collator name can't be empty")).into(), - ), - ); - } - } - self.with_parachain(|parachain| { let mut parachain_config = parachain.with_id(id).with_collator(|collator| { collator - .with_name( - collator_names - .first() - .expect("collator names list is not empty"), - ) + .with_name(collator_names.first().unwrap_or(&"".to_string())) .validator(true) }); @@ -1566,7 +1532,7 @@ mod tests { assert_eq!( errors.first().unwrap().to_string(), - "relaychain.relaychain name can't be empty" + "relaychain.chain: can't be empty" ); } @@ -1578,7 +1544,7 @@ mod tests { assert_eq!( errors.first().unwrap().to_string(), - "relaychain.node names list can't be empty" + "relaychain.nodes[''].name: can't be empty" ); } @@ -1593,7 +1559,7 @@ mod tests { assert_eq!( errors.first().unwrap().to_string(), - "relaychain.node name can't be empty" + "relaychain.nodes[''].name: can't be empty" ); } @@ -1643,7 +1609,7 @@ mod tests { assert_eq!( errors.first().unwrap().to_string(), - "parachain[1].collator names list can't be empty" + "parachain[1].can't be empty" ); } @@ -1657,7 +1623,7 @@ mod tests { assert_eq!( errors.first().unwrap().to_string(), - "parachain[1].collator name can't be empty" + "parachain[1].collators[''].name: can't be empty" ); } }