feat: catch dot port missing early (#4082)

* feat: catch dot port missing early

* refactor: use closure to avoid duplication

* tests: test dot port validation

* refactor: use regex instead of Url

* refactor: dedup map_err

engine/Cargo.toml

@@ -88,6 +88,7 @@ zmq = { git = "https://github.com/chainflip-io/rust-zmq.git", tag = "chainflip-v
   "vendored",
 ] }
 warp = { version = "0.3.6" }
+regex = { version = "1"}
 
 # Local deps
 cf-chains = { path = "../state-chain/chains" }

engine/config/testing/config/Settings.toml

@@ -13,8 +13,8 @@ ws_endpoint = "ws://localhost:8545"
 
 [dot.rpc]
 # NB: You will need to manually add :443 to the url provided by the provider, as jsonrpsee wants one.
-ws_endpoint = "wss://my_fake_polkadot_rpc:443/<secret_key>"
-http_endpoint = "http://my_fake_polkadot_rpc:443/<secret_key>"
+ws_endpoint = "wss://my_fake_polkadot_rpc:443/secret_key"
+http_endpoint = "http://my_fake_polkadot_rpc:443/secret_key"
 
 [btc.rpc]
 http_endpoint = "http://localhost:18443"

engine/src/settings.rs

@@ -11,6 +11,7 @@ use config::{Config, ConfigBuilder, ConfigError, Environment, File, Map, Source,
 use serde::{de, Deserialize, Deserializer};
 
 pub use anyhow::Result;
+use regex::Regex;
 use sp_runtime::DeserializeOwned;
 use url::Url;
 
@@ -106,8 +107,35 @@ pub struct Dot {
 
 impl Dot {
 	pub fn validate_settings(&self) -> Result<(), ConfigError> {
-		self.nodes.validate()
+		self.nodes.validate()?;
+
+		// Check that all endpoints have a port number
+		let validate_dot_endpoints = |endpoints: &WsHttpEndpoints| -> Result<(), ConfigError> {
+			validate_port_exists(&endpoints.ws_endpoint)
+				.and_then(|_| validate_port_exists(&endpoints.http_endpoint))
+				.map_err(|e| {
+					ConfigError::Message(format!(
+						"Polkadot node endpoints must include a port number: {e}"
+					))
+				})
+		};
+		validate_dot_endpoints(&self.nodes.primary)?;
+		if let Some(backup) = &self.nodes.backup {
+			validate_dot_endpoints(backup)?;
+		}
+		Ok(())
+	}
+}
+
+// Checks that the url has a port number
+fn validate_port_exists(url: &SecretUrl) -> Result<()> {
+	// NB: We are using regex instead of Url because Url.port() returns None for wss/https urls with
+	// default ports.
+	let re = Regex::new(r":([0-9]+)").unwrap();
+	if re.captures(url.as_ref()).is_none() {
+		bail!("No port found in url: {url}");
 	}
+	Ok(())
 }
 
 #[derive(Debug, Deserialize, Clone, Default, PartialEq, Eq)]
@@ -992,4 +1020,33 @@ pub mod tests {
 		assert!(is_valid_db_path(Path::new("data.errdb")).is_err());
 		assert!(is_valid_db_path(Path::new("thishasnoextension")).is_err());
 	}
+
+	#[test]
+	fn test_dot_port_validation() {
+		let valid_settings = Dot {
+			nodes: NodeContainer {
+				primary: WsHttpEndpoints {
+					ws_endpoint: "wss://valid.endpoint_with_port:443/secret_key".into(),
+					http_endpoint: "https://valid.endpoint_with_port:443/secret_key".into(),
+				},
+				backup: Some(WsHttpEndpoints {
+					ws_endpoint: "ws://valid.endpoint_with_port:1234".into(),
+					http_endpoint: "http://valid.endpoint_with_port:6969".into(),
+				}),
+			},
+		};
+		assert_ok!(valid_settings.validate_settings());
+
+		let mut invalid_primary_settings = valid_settings.clone();
+		invalid_primary_settings.nodes.primary.ws_endpoint =
+			"ws://invalid.no_port_in_url/secret_key".into();
+		assert!(invalid_primary_settings.validate_settings().is_err());
+
+		let mut invalid_backup_settings = valid_settings.clone();
+		invalid_backup_settings.nodes.backup = Some(WsHttpEndpoints {
+			ws_endpoint: "ws://valid.endpoint_with_port:443".into(),
+			http_endpoint: "http://invalid.no_port_in_url/secret_key".into(),
+		});
+		assert!(invalid_backup_settings.validate_settings().is_err());
+	}
 }