Remove Default impls for TpuConnectionCache and ConnectionPool clients (#28788)

* Remove TpuConnectionCache impl Default

* Add fallible ctors for Quic/UdpClients

* Add Quic/TpuClient ctor trait

* Remove Quic/UdpClient impl Default

Cargo.lock

@@ -5963,6 +5963,7 @@ dependencies = [
  "solana-sdk 1.15.0",
  "solana-streamer",
  "solana-tpu-client",
+ "thiserror",
 ]
 
 [[package]]
@@ -6654,6 +6655,7 @@ version = "1.15.0"
 dependencies = [
  "solana-net-utils",
  "solana-tpu-client",
+ "thiserror",
 ]
 
 [[package]]

quic-client/Cargo.toml

@@ -14,6 +14,7 @@ solana-metrics = { path = "../metrics", version = "=1.15.0" }
 solana-sdk = { path = "../sdk", version = "=1.15.0" }
 solana-streamer = { path = "../streamer", version = "=1.15.0" }
 solana-tpu-client = { path = "../tpu-client", version = "=1.15.0" }
+thiserror = "1.0"
 
 [dev-dependencies]
 solana-logger = { path = "../logger", version = "=1.15.0" }

quic-client/src/lib.rs

@@ -19,15 +19,24 @@ use {
     },
     solana_tpu_client::{
         connection_cache_stats::ConnectionCacheStats,
-        tpu_connection_cache::{BaseTpuConnection, ConnectionPool, ConnectionPoolError},
+        tpu_connection_cache::{
+            BaseTpuConnection, ConnectionPool, ConnectionPoolError, NewTpuConfig,
+        },
     },
     std::{
         error::Error,
         net::{IpAddr, Ipv4Addr, SocketAddr},
         sync::{Arc, RwLock},
     },
+    thiserror::Error,
 };
 
+#[derive(Error, Debug)]
+pub enum QuicClientError {
+    #[error("Certificate error: {0}")]
+    CertificateError(String),
+}
+
 pub struct QuicPool {
     connections: Vec<Arc<Quic>>,
     endpoint: Arc<QuicLazyInitializedEndpoint>,
@@ -81,21 +90,23 @@ pub struct QuicConfig {
     maybe_client_pubkey: Option<Pubkey>,
 }
 
-impl Default for QuicConfig {
-    fn default() -> Self {
+impl NewTpuConfig for QuicConfig {
+    type ClientError = QuicClientError;
+
+    fn new() -> Result<Self, QuicClientError> {
         let (certs, priv_key) = new_self_signed_tls_certificate_chain(
             &Keypair::new(),
             IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)),
         )
-        .expect("Failed to initialize QUIC client certificates");
-        Self {
+        .map_err(|err| QuicClientError::CertificateError(err.to_string()))?;
+        Ok(Self {
             client_certificate: Arc::new(QuicClientCertificate {
                 certificates: certs,
                 key: priv_key,
             }),
             maybe_staked_nodes: None,
             maybe_client_pubkey: None,
-        }
+        })
     }
 }
 
@@ -177,13 +188,16 @@ mod tests {
             QUIC_MAX_UNSTAKED_CONCURRENT_STREAMS, QUIC_MIN_STAKED_CONCURRENT_STREAMS,
             QUIC_TOTAL_STAKED_CONCURRENT_STREAMS,
         },
-        solana_tpu_client::tpu_connection_cache::TpuConnectionCache,
+        solana_tpu_client::tpu_connection_cache::{
+            TpuConnectionCache, DEFAULT_TPU_CONNECTION_POOL_SIZE,
+        },
     };
 
     #[test]
     fn test_connection_cache_max_parallel_chunks() {
         solana_logger::setup();
-        let connection_cache = TpuConnectionCache::<QuicPool>::default();
+        let connection_cache =
+            TpuConnectionCache::<QuicPool>::new(DEFAULT_TPU_CONNECTION_POOL_SIZE).unwrap();
         let mut tpu_config = connection_cache.tpu_config;
         assert_eq!(
             tpu_config.compute_max_parallel_streams(),

tpu-client/src/tpu_connection_cache.rs

@@ -36,10 +36,20 @@ pub struct TpuConnectionCache<P: ConnectionPool> {
 }
 
 impl<P: ConnectionPool> TpuConnectionCache<P> {
-    pub fn new(connection_pool_size: usize) -> Self {
+    pub fn new(
+        connection_pool_size: usize,
+    ) -> Result<Self, <P::TpuConfig as NewTpuConfig>::ClientError> {
+        let config = P::TpuConfig::new()?;
+        Ok(Self::new_with_config(connection_pool_size, config))
+    }
+
+    pub fn new_with_config(connection_pool_size: usize, tpu_config: P::TpuConfig) -> Self {
         Self {
+            map: RwLock::new(IndexMap::with_capacity(MAX_CONNECTIONS)),
+            stats: Arc::new(ConnectionCacheStats::default()),
+            last_stats: AtomicInterval::default(),
             connection_pool_size: 1.max(connection_pool_size), // The minimum pool size is 1.
-            ..Self::default()
+            tpu_config,
         }
     }
 
@@ -245,27 +255,22 @@ impl<P: ConnectionPool> TpuConnectionCache<P> {
     }
 }
 
-impl<P: ConnectionPool> Default for TpuConnectionCache<P> {
-    fn default() -> Self {
-        Self {
-            map: RwLock::new(IndexMap::with_capacity(MAX_CONNECTIONS)),
-            stats: Arc::new(ConnectionCacheStats::default()),
-            last_stats: AtomicInterval::default(),
-            connection_pool_size: DEFAULT_TPU_CONNECTION_POOL_SIZE,
-            tpu_config: P::TpuConfig::default(),
-        }
-    }
-}
-
 #[derive(Error, Debug)]
 pub enum ConnectionPoolError {
     #[error("connection index is out of range of the pool")]
     IndexOutOfRange,
 }
 
+pub trait NewTpuConfig {
+    type ClientError;
+    fn new() -> Result<Self, Self::ClientError>
+    where
+        Self: Sized;
+}
+
 pub trait ConnectionPool {
     type PoolTpuConnection: BaseTpuConnection;
-    type TpuConfig: Default;
+    type TpuConfig: NewTpuConfig;
     const PORT_OFFSET: u16 = 0;
 
     /// Create a new connection pool based on protocol-specific configuration
@@ -412,6 +417,19 @@ mod tests {
         }
     }
 
+    impl NewTpuConfig for MockUdpConfig {
+        type ClientError = String;
+
+        fn new() -> Result<Self, String> {
+            Ok(Self {
+                tpu_udp_socket: Arc::new(
+                    solana_net_utils::bind_with_any_port(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)))
+                        .map_err(|_| "Unable to bind to UDP socket".to_string())?,
+                ),
+            })
+        }
+    }
+
     pub struct MockUdp(Arc<UdpSocket>);
     impl BaseTpuConnection for MockUdp {
         type BlockingConnectionType = MockUdpTpuConnection;
@@ -508,7 +526,8 @@ mod tests {
         // we can actually connect to those addresses - TPUConnection implementations should either
         // be lazy and not connect until first use or handle connection errors somehow
         // (without crashing, as would be required in a real practical validator)
-        let connection_cache = TpuConnectionCache::<MockUdpPool>::default();
+        let connection_cache =
+            TpuConnectionCache::<MockUdpPool>::new(DEFAULT_TPU_CONNECTION_POOL_SIZE).unwrap();
         let port_offset = MOCK_PORT_OFFSET;
         let addrs = (0..MAX_CONNECTIONS)
             .into_iter()
@@ -555,7 +574,7 @@ mod tests {
         let port = u16::MAX - MOCK_PORT_OFFSET + 1;
         assert!(port.checked_add(MOCK_PORT_OFFSET).is_none());
         let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), port);
-        let connection_cache = TpuConnectionCache::<MockUdpPool>::new(1);
+        let connection_cache = TpuConnectionCache::<MockUdpPool>::new(1).unwrap();
 
         let conn: MockUdpTpuConnection = connection_cache.get_connection(&addr);
         // We (intentionally) don't have an interface that allows us to distinguish between

udp-client/Cargo.toml

@@ -12,3 +12,4 @@ edition = "2021"
 [dependencies]
 solana-net-utils = { path = "../net-utils", version = "=1.15.0" }
 solana-tpu-client = { path = "../tpu-client", version = "=1.15.0" }
+thiserror = "1.0"

udp-client/src/lib.rs

@@ -10,14 +10,23 @@ use {
     },
     solana_tpu_client::{
         connection_cache_stats::ConnectionCacheStats,
-        tpu_connection_cache::{BaseTpuConnection, ConnectionPool, ConnectionPoolError},
+        tpu_connection_cache::{
+            BaseTpuConnection, ConnectionPool, ConnectionPoolError, NewTpuConfig,
+        },
     },
     std::{
         net::{IpAddr, Ipv4Addr, SocketAddr, UdpSocket},
         sync::Arc,
     },
+    thiserror::Error,
 };
 
+#[derive(Error, Debug)]
+pub enum UdpClientError {
+    #[error("IO error: {0:?}")]
+    IoError(#[from] std::io::Error),
+}
+
 pub struct UdpPool {
     connections: Vec<Arc<Udp>>,
 }
@@ -62,14 +71,15 @@ pub struct UdpConfig {
     tpu_udp_socket: Arc<UdpSocket>,
 }
 
-impl Default for UdpConfig {
-    fn default() -> Self {
-        Self {
-            tpu_udp_socket: Arc::new(
-                solana_net_utils::bind_with_any_port(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)))
-                    .expect("Unable to bind to UDP socket"),
-            ),
-        }
+impl NewTpuConfig for UdpConfig {
+    type ClientError = UdpClientError;
+
+    fn new() -> Result<Self, UdpClientError> {
+        let socket = solana_net_utils::bind_with_any_port(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)))
+            .map_err(Into::<UdpClientError>::into)?;
+        Ok(Self {
+            tpu_udp_socket: Arc::new(socket),
+        })
     }
 }