From 6bf355cc93fd7c68c72b6790d1b1dd437e2d2170 Mon Sep 17 00:00:00 2001 From: Chris O'Neil Date: Wed, 24 Jul 2024 14:55:12 +0100 Subject: [PATCH] Revert "feat!: limit error surface" This reverts commit b17a53615a574c3d8dc075669c24f6b8c0eb0ebc. --- sn_client/src/api.rs | 19 ++++++------------- sn_client/src/error.rs | 3 +++ sn_networking/src/driver.rs | 33 ++++++++++----------------------- sn_networking/src/error.rs | 7 ++++--- 4 files changed, 23 insertions(+), 39 deletions(-) diff --git a/sn_client/src/api.rs b/sn_client/src/api.rs index 05ec3e67ff..f7aaf74d04 100644 --- a/sn_client/src/api.rs +++ b/sn_client/src/api.rs @@ -57,13 +57,6 @@ pub const CONNECTION_TIMEOUT: Duration = Duration::from_secs(30); /// The timeout duration for the client to receive any response from the network. const INACTIVITY_TIMEOUT: Duration = Duration::from_secs(30); -// Init during compilation, instead of runtime error that should never happen -// Option::expect will be stabilised as const in the future (https://github.com/rust-lang/rust/issues/67441) -const QUORUM_2: NonZeroUsize = match NonZeroUsize::new(2) { - Some(v) => v, - None => panic!("2 is not be zero"), -}; - impl Client { /// A quick client with a random secret key and some peers. pub async fn quick_start(peers: Option>) -> Result { @@ -121,9 +114,7 @@ impl Client { #[cfg(feature = "open-metrics")] network_builder.metrics_registry(Some(Registry::default())); - let (network, mut network_event_receiver, swarm_driver) = network_builder - .build_client() - .map_err(NetworkError::MdnsBuildBehaviourError)?; + let (network, mut network_event_receiver, swarm_driver) = network_builder.build_client()?; info!("Client constructed network and swarm_driver"); // If the events broadcaster is not provided by the caller, then we create a new one. @@ -459,7 +450,7 @@ impl Client { ) -> Result { let key = NetworkAddress::from_register_address(address).to_record_key(); let get_quorum = if is_verifying { - Quorum::N(QUORUM_2) + Quorum::N(NonZeroUsize::new(2).ok_or(Error::NonZeroUsizeWasInitialisedAsZero)?) } else { Quorum::One }; @@ -660,7 +651,9 @@ impl Client { let verification = if verify_store { let verification_cfg = GetRecordCfg { - get_quorum: Quorum::N(QUORUM_2), + get_quorum: Quorum::N( + NonZeroUsize::new(2).ok_or(Error::NonZeroUsizeWasInitialisedAsZero)?, + ), retry_strategy, target_record: None, // Not used since we use ChunkProof expected_holders: Default::default(), @@ -775,7 +768,7 @@ impl Client { address.clone(), random_nonce, expected_proof, - Quorum::N(QUORUM_2), + Quorum::N(NonZeroUsize::new(2).ok_or(Error::NonZeroUsizeWasInitialisedAsZero)?), None, ) .await diff --git a/sn_client/src/error.rs b/sn_client/src/error.rs index 50ce6525e1..adbe0ef884 100644 --- a/sn_client/src/error.rs +++ b/sn_client/src/error.rs @@ -90,6 +90,9 @@ pub enum Error { #[error("Total price exceed possible token amount")] TotalPriceTooHigh, + #[error("Logic error: NonZeroUsize was initialised as zero")] + NonZeroUsizeWasInitialisedAsZero, + #[error("Could not connect to the network in {0:?}")] ConnectionTimeout(Duration), diff --git a/sn_networking/src/driver.rs b/sn_networking/src/driver.rs index ebc56fc29b..157dc785a0 100644 --- a/sn_networking/src/driver.rs +++ b/sn_networking/src/driver.rs @@ -117,13 +117,6 @@ const NETWORKING_CHANNEL_SIZE: usize = 10_000; /// Time before a Kad query times out if no response is received const KAD_QUERY_TIMEOUT_S: Duration = Duration::from_secs(10); -// Init during compilation, instead of runtime error that should never happen -// Option::expect will be stabilised as const in the future (https://github.com/rust-lang/rust/issues/67441) -const REPLICATION_FACTOR: NonZeroUsize = match NonZeroUsize::new(CLOSE_GROUP_SIZE) { - Some(v) => v, - None => panic!("CLOSE_GROUP_SIZE should not be zero"), -}; - /// The various settings to apply to when fetching a record from network #[derive(Clone)] pub struct GetRecordCfg { @@ -311,7 +304,10 @@ impl NetworkBuilder { // 1mb packet size .set_max_packet_size(MAX_PACKET_SIZE) // How many nodes _should_ store data. - .set_replication_factor(REPLICATION_FACTOR) + .set_replication_factor( + NonZeroUsize::new(CLOSE_GROUP_SIZE) + .ok_or_else(|| NetworkError::InvalidCloseGroupSize)?, + ) .set_query_timeout(KAD_QUERY_TIMEOUT_S) // Require iterative queries to use disjoint paths for increased resiliency in the presence of potentially adversarial nodes. .disjoint_query_paths(true) @@ -380,12 +376,7 @@ impl NetworkBuilder { } /// Same as `build_node` API but creates the network components in client mode - pub fn build_client( - self, - ) -> std::result::Result< - (Network, mpsc::Receiver, SwarmDriver), - MdnsBuildBehaviourError, - > { + pub fn build_client(self) -> Result<(Network, mpsc::Receiver, SwarmDriver)> { // Create a Kademlia behaviour for client mode, i.e. set req/resp protocol // to outbound-only mode and don't listen on any address let mut kad_cfg = kad::Config::default(); // default query timeout is 60 secs @@ -397,7 +388,10 @@ impl NetworkBuilder { // Require iterative queries to use disjoint paths for increased resiliency in the presence of potentially adversarial nodes. .disjoint_query_paths(true) // How many nodes _should_ store data. - .set_replication_factor(REPLICATION_FACTOR); + .set_replication_factor( + NonZeroUsize::new(CLOSE_GROUP_SIZE) + .ok_or_else(|| NetworkError::InvalidCloseGroupSize)?, + ); let (network, net_event_recv, driver) = self.build( kad_cfg, @@ -421,10 +415,7 @@ impl NetworkBuilder { req_res_protocol: ProtocolSupport, identify_version: String, #[cfg(feature = "upnp")] upnp: bool, - ) -> std::result::Result< - (Network, mpsc::Receiver, SwarmDriver), - MdnsBuildBehaviourError, - > { + ) -> Result<(Network, mpsc::Receiver, SwarmDriver)> { let peer_id = PeerId::from(self.keypair.public()); // vdash metric (if modified please notify at https://github.com/happybeing/vdash/issues): #[cfg(not(target_arch = "wasm32"))] @@ -894,7 +885,3 @@ impl SwarmDriver { Ok(()) } } - -#[derive(Debug, thiserror::Error)] -#[error("building the mDNS behaviour failed: {0}")] -pub struct MdnsBuildBehaviourError(#[from] std::io::Error); diff --git a/sn_networking/src/error.rs b/sn_networking/src/error.rs index 5b413547ce..103a79a9a3 100644 --- a/sn_networking/src/error.rs +++ b/sn_networking/src/error.rs @@ -24,8 +24,6 @@ use thiserror::Error; use tokio::sync::oneshot; use xor_name::XorName; -use crate::driver::MdnsBuildBehaviourError; - pub(super) type Result = std::result::Result; /// GetRecord Query errors @@ -157,6 +155,9 @@ pub enum NetworkError { #[error("Could not get enough peers ({required}) to satisfy the request, found {found}")] NotEnoughPeers { found: usize, required: usize }, + #[error("Close group size must be a non-zero usize")] + InvalidCloseGroupSize, + #[error("Node Listen Address was not provided during construction")] ListenAddressNotProvided, @@ -184,7 +185,7 @@ pub enum NetworkError { OutgoingResponseDropped(Response), #[error("Error setting up behaviour: {0}")] - MdnsBuildBehaviourError(#[from] MdnsBuildBehaviourError), + BahviourErr(String), } #[cfg(test)]