Skip to content

Commit c952f53

Browse files
Use url::Url instead of string across sncast (#4017)
<!-- Reference any GitHub issues resolved by this PR --> **Stack** - #4020 - #4019 - #4017 ⬅ - #4013 Closes # ## Introduced changes <!-- A brief description of the changes --> - ## Checklist <!-- Make sure all of these are complete --> - [ ] Linked relevant issue - [ ] Updated relevant documentation - [x] Added relevant tests - [x] Performed self-review of the code - [ ] Added changes to `CHANGELOG.md`
1 parent b79f597 commit c952f53

File tree

23 files changed

+152
-139
lines changed

23 files changed

+152
-139
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/configuration/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,6 @@ serde.workspace = true
1313
camino.workspace = true
1414
toml.workspace = true
1515
tempfile.workspace = true
16+
17+
[dev-dependencies]
18+
url.workspace = true

crates/configuration/src/lib.rs

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,12 @@ pub fn find_config_file() -> Result<Utf8PathBuf> {
7979

8080
#[cfg(test)]
8181
mod tests {
82-
use std::fs::{self, File};
83-
8482
use super::*;
8583
use crate::test_utils::copy_config_to_tempdir;
8684
use serde::{Deserialize, Serialize};
85+
use std::fs::{self, File};
8786
use tempfile::tempdir;
87+
use url::Url;
8888

8989
#[test]
9090
fn find_config_in_current_dir() {
@@ -151,8 +151,7 @@ mod tests {
151151

152152
#[derive(Debug, Default, Serialize, Deserialize)]
153153
pub struct StubConfig {
154-
#[serde(default)]
155-
pub url: String,
154+
pub url: Option<Url>,
156155
#[serde(default)]
157156
pub account: String,
158157
}
@@ -162,7 +161,7 @@ mod tests {
162161
}
163162

164163
fn from_raw(config: serde_json::Value) -> Result<Self> {
165-
Ok(serde_json::from_value::<StubConfig>(config)?)
164+
Ok(serde_json::from_value::<Self>(config)?)
166165
}
167166
}
168167
#[test]
@@ -174,7 +173,10 @@ mod tests {
174173
)
175174
.unwrap();
176175
assert_eq!(config.account, String::from("user3"));
177-
assert_eq!(config.url, String::from("http://127.0.0.1:5050/rpc"));
176+
assert_eq!(
177+
config.url,
178+
Some(Url::parse("http://127.0.0.1:5050/rpc").unwrap())
179+
);
178180
}
179181

180182
#[test]
@@ -186,7 +188,24 @@ mod tests {
186188
)
187189
.unwrap();
188190
assert_eq!(config.account, String::from("user1"));
189-
assert_eq!(config.url, String::from("http://127.0.0.1:5055/rpc"));
191+
assert_eq!(
192+
config.url,
193+
Some(Url::parse("http://127.0.0.1:5055/rpc").unwrap())
194+
);
195+
}
196+
#[test]
197+
fn load_config_invalid_url() {
198+
let tempdir = copy_config_to_tempdir("tests/data/stubtool_snfoundry.toml", None);
199+
let err = load_config::<StubConfig>(
200+
Some(&Utf8PathBuf::try_from(tempdir.path().to_path_buf()).unwrap()),
201+
Some(&String::from("profile6")),
202+
)
203+
.unwrap_err();
204+
205+
assert!(
206+
err.to_string()
207+
.contains("relative URL without a base: \"invalid_url\"")
208+
);
190209
}
191210

192211
#[test]
@@ -199,7 +218,7 @@ mod tests {
199218
.unwrap();
200219

201220
assert_eq!(config.account, String::new());
202-
assert_eq!(config.url, String::new());
221+
assert_eq!(config.url, None);
203222
}
204223

205224
#[derive(Debug, Default, Serialize, Deserialize)]
@@ -231,7 +250,7 @@ mod tests {
231250
}
232251

233252
fn from_raw(config: serde_json::Value) -> Result<Self> {
234-
Ok(serde_json::from_value::<StubComplexConfig>(config)?)
253+
Ok(serde_json::from_value::<Self>(config)?)
235254
}
236255
}
237256

crates/configuration/tests/data/stubtool_snfoundry.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,15 @@ account = "user3"
2424

2525
[stubtool.profile5]
2626
url = "http://127.0.0.1:5055/rpc"
27+
28+
[stubtool.profile6]
29+
url = "invalid_url"
2730
account = "user8"
2831

2932
[stubtool.with-envs]
3033
url = "$VALUE_STRING123132"
3134
account = "$VALUE_INT123132"
35+
3236
[stubtool.with-envs.nested]
3337
list-example = [ "$VALUE_BOOL1231321", "$VALUE_BOOL1231322" ]
3438
url-nested = "$VALUE_FLOAT123132"

crates/forge/src/warn.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ pub(crate) async fn warn_if_incompatible_rpc_version(
3535
for url in urls {
3636
let ui = ui.clone();
3737
handles.push(tokio::spawn(async move {
38-
let client = create_rpc_client(url.as_ref())?;
38+
let client = create_rpc_client(&url)?;
3939

4040
verify_and_warn_if_incompatible_rpc_version(&client, &url, &ui).await
4141
}));

crates/shared/src/rpc.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@ use starknet_rust::providers::{JsonRpcClient, Provider};
77
use std::str::FromStr;
88
use url::Url;
99

10-
pub fn create_rpc_client(url: &str) -> Result<JsonRpcClient<HttpTransport>> {
11-
let parsed_url = Url::parse(url).with_context(|| format!("Failed to parse URL: {url}"))?;
12-
let client = JsonRpcClient::new(HttpTransport::new(parsed_url));
10+
pub fn create_rpc_client(url: &Url) -> Result<JsonRpcClient<HttpTransport>> {
11+
let client = JsonRpcClient::new(HttpTransport::new(url.clone()));
1312
Ok(client)
1413
}
1514

crates/sncast/src/helpers/account.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use starknet_rust::{
1010
};
1111
use std::collections::{HashMap, HashSet};
1212
use std::fs;
13+
use url::Url;
1314

1415
use crate::{AccountData, read_and_parse_json_file};
1516
use anyhow::Context;
@@ -82,14 +83,14 @@ pub fn is_devnet_account(account: &str) -> bool {
8283
pub async fn get_account_from_devnet<'a>(
8384
account: &str,
8485
provider: &'a JsonRpcClient<HttpTransport>,
85-
url: &str,
86+
url: &Url,
8687
) -> Result<SingleOwnerAccount<&'a JsonRpcClient<HttpTransport>, LocalWallet>> {
8788
let account_number: u8 = account
8889
.strip_prefix("devnet-")
8990
.map(|s| s.parse::<u8>().expect("Invalid devnet account number"))
9091
.context("Failed to parse devnet account number")?;
9192

92-
let devnet_provider = DevnetProvider::new(url);
93+
let devnet_provider = DevnetProvider::new(url.as_ref());
9394
devnet_provider.ensure_alive().await?;
9495

9596
let devnet_config = devnet_provider.get_config().await;

crates/sncast/src/helpers/configuration.rs

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use anyhow::Result;
44
use camino::Utf8PathBuf;
55
use configuration::Config;
66
use serde::{Deserialize, Serialize};
7+
use url::Url;
78

89
#[must_use]
910
pub const fn show_explorer_links_default() -> bool {
@@ -13,14 +14,14 @@ pub const fn show_explorer_links_default() -> bool {
1314
#[derive(Deserialize, Serialize, Clone, Debug, PartialEq, Default)]
1415
#[serde(deny_unknown_fields)]
1516
pub struct NetworksConfig {
16-
pub mainnet: Option<String>,
17-
pub sepolia: Option<String>,
18-
pub devnet: Option<String>,
17+
pub mainnet: Option<Url>,
18+
pub sepolia: Option<Url>,
19+
pub devnet: Option<Url>,
1920
}
2021

2122
impl NetworksConfig {
2223
#[must_use]
23-
pub fn get_url(&self, network: Network) -> Option<&String> {
24+
pub fn get_url(&self, network: Network) -> Option<&Url> {
2425
match network {
2526
Network::Mainnet => self.mainnet.as_ref(),
2627
Network::Sepolia => self.sepolia.as_ref(),
@@ -45,7 +46,7 @@ impl NetworksConfig {
4546
pub struct CastConfig {
4647
#[serde(default)]
4748
/// RPC url
48-
pub url: String,
49+
pub url: Option<Url>,
4950

5051
#[serde(default)]
5152
pub account: String,
@@ -86,7 +87,7 @@ pub struct CastConfig {
8687
impl Default for CastConfig {
8788
fn default() -> Self {
8889
Self {
89-
url: String::default(),
90+
url: None,
9091
account: String::default(),
9192
accounts_file: Utf8PathBuf::default(),
9293
keystore: None,
@@ -104,7 +105,7 @@ impl Config for CastConfig {
104105
}
105106

106107
fn from_raw(config: serde_json::Value) -> Result<Self> {
107-
Ok(serde_json::from_value::<CastConfig>(config)?)
108+
Ok(serde_json::from_value::<Self>(config)?)
108109
}
109110
}
110111

@@ -115,34 +116,34 @@ mod tests {
115116
#[test]
116117
fn test_networks_config_get() {
117118
let networks = NetworksConfig {
118-
mainnet: Some("https://mainnet.example.com".to_string()),
119-
sepolia: Some("https://sepolia.example.com".to_string()),
120-
devnet: Some("https://devnet.example.com".to_string()),
119+
mainnet: Some(Url::parse("https://mainnet.example.com").unwrap()),
120+
sepolia: Some(Url::parse("https://sepolia.example.com").unwrap()),
121+
devnet: Some(Url::parse("https://devnet.example.com").unwrap()),
121122
};
122123

123124
assert_eq!(
124125
networks.get_url(Network::Mainnet),
125-
Some(&"https://mainnet.example.com".to_string())
126+
Some(&Url::parse("https://mainnet.example.com").unwrap())
126127
);
127128
assert_eq!(
128129
networks.get_url(Network::Sepolia),
129-
Some(&"https://sepolia.example.com".to_string())
130+
Some(&Url::parse("https://sepolia.example.com").unwrap())
130131
);
131132
assert_eq!(
132133
networks.get_url(Network::Devnet),
133-
Some(&"https://devnet.example.com".to_string())
134+
Some(&Url::parse("https://devnet.example.com").unwrap())
134135
);
135136
}
136137

137138
#[test]
138139
fn test_networks_config_override() {
139140
let mut global = NetworksConfig {
140-
mainnet: Some("https://global-mainnet.example.com".to_string()),
141-
sepolia: Some("https://global-sepolia.example.com".to_string()),
141+
mainnet: Some(Url::parse("https://global-mainnet.example.com").unwrap()),
142+
sepolia: Some(Url::parse("https://global-sepolia.example.com").unwrap()),
142143
devnet: None,
143144
};
144145
let local = NetworksConfig {
145-
mainnet: Some("https://local-mainnet.example.com".to_string()),
146+
mainnet: Some(Url::parse("https://local-mainnet.example.com").unwrap()),
146147
sepolia: None,
147148
devnet: None,
148149
};
@@ -152,12 +153,12 @@ mod tests {
152153
// Local mainnet should override global
153154
assert_eq!(
154155
global.mainnet,
155-
Some("https://local-mainnet.example.com".to_string())
156+
Some(Url::parse("https://local-mainnet.example.com").unwrap())
156157
);
157158
// Global sepolia should remain
158159
assert_eq!(
159160
global.sepolia,
160-
Some("https://global-sepolia.example.com".to_string())
161+
Some(Url::parse("https://global-sepolia.example.com").unwrap())
161162
);
162163
}
163164

crates/sncast/src/helpers/devnet/detection.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
mod direct;
22
mod docker;
33
mod flag_parsing;
4-
54
use std::process::Command;
5+
use url::Url;
66

77
use crate::helpers::devnet::provider::DevnetProvider;
88

@@ -31,9 +31,11 @@ pub enum DevnetDetectionError {
3131
"Found starknet-devnet process, but could not reach it. Please use `--url <URL>` to specify the correct URL."
3232
)]
3333
ProcessNotReachable,
34+
#[error("Failed to parse devnet URL: {0}")]
35+
InvalidUrl(#[from] url::ParseError),
3436
}
3537

36-
pub async fn detect_devnet_url() -> Result<String, DevnetDetectionError> {
38+
pub async fn detect_devnet_url() -> Result<Url, DevnetDetectionError> {
3739
detect_devnet_from_processes().await
3840
}
3941

@@ -42,21 +44,21 @@ pub async fn is_devnet_running() -> bool {
4244
detect_devnet_from_processes().await.is_ok()
4345
}
4446

45-
async fn detect_devnet_from_processes() -> Result<String, DevnetDetectionError> {
47+
async fn detect_devnet_from_processes() -> Result<Url, DevnetDetectionError> {
4648
match find_devnet_process_info() {
4749
Ok(info) => {
4850
if is_devnet_url_reachable(&info.host, info.port).await {
49-
Ok(format!("http://{}:{}", info.host, info.port))
51+
Ok(Url::parse(&format!("http://{}:{}", info.host, info.port))?)
5052
} else {
5153
Err(DevnetDetectionError::ProcessNotReachable)
5254
}
5355
}
5456
Err(DevnetDetectionError::NoInstance | DevnetDetectionError::CommandFailed) => {
5557
// Fallback to default starknet-devnet URL if reachable
5658
if is_devnet_url_reachable(DEFAULT_DEVNET_HOST, DEFAULT_DEVNET_PORT).await {
57-
Ok(format!(
59+
Ok(Url::parse(&format!(
5860
"http://{DEFAULT_DEVNET_HOST}:{DEFAULT_DEVNET_PORT}"
59-
))
61+
))?)
6062
} else {
6163
Err(DevnetDetectionError::NoInstance)
6264
}

0 commit comments

Comments
 (0)