-
Notifications
You must be signed in to change notification settings - Fork 239
Support devnet in --network
flag
#3786
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 22 commits
120b679
61b534d
807dcba
9a1ea80
dcbb672
7d95b14
0e04f81
a5510dd
a18a02d
7299ae0
f4b31ea
2d0aa6c
eafe1ca
88071ee
a9b1b51
016fc2e
7793096
4613458
7aaf4d4
f73163e
f8c1ac6
71357ba
ed83201
079e8db
3513d28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,309 @@ | ||||||||||||||
use std::process::Command; | ||||||||||||||
|
||||||||||||||
const DEFAULT_DEVNET_HOST: &str = "127.0.0.1"; | ||||||||||||||
const DEFAULT_DEVNET_PORT: u16 = 5050; | ||||||||||||||
|
||||||||||||||
#[derive(Debug, Clone)] | ||||||||||||||
struct DevnetProcessInfo { | ||||||||||||||
host: String, | ||||||||||||||
port: u16, | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
#[derive(Debug)] | ||||||||||||||
enum DevnetDetectionError { | ||||||||||||||
NoInstance, | ||||||||||||||
MultipleInstances, | ||||||||||||||
CommandFailed, | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
pub fn detect_devnet_url() -> Result<String, String> { | ||||||||||||||
detect_devnet_from_processes() | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
#[must_use] | ||||||||||||||
pub fn is_devnet_running() -> bool { | ||||||||||||||
detect_devnet_from_processes().is_ok() | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn detect_devnet_from_processes() -> Result<String, String> { | ||||||||||||||
match find_devnet_process_info() { | ||||||||||||||
Ok(info) => Ok(format!("http://{}:{}", info.host, info.port)), | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could probably do this
Suggested change
|
||||||||||||||
Err(DevnetDetectionError::MultipleInstances) => { | ||||||||||||||
Err("Multiple starknet-devnet instances found. Please use `--url <URL>` to specify which one to use.".to_string()) | ||||||||||||||
} | ||||||||||||||
Err(DevnetDetectionError::NoInstance | DevnetDetectionError::CommandFailed) => { | ||||||||||||||
// Fallback to default starknet-devnet URL if reachable | ||||||||||||||
if is_port_reachable(DEFAULT_DEVNET_HOST, DEFAULT_DEVNET_PORT) { | ||||||||||||||
Ok(format!("http://{DEFAULT_DEVNET_HOST}:{DEFAULT_DEVNET_PORT}")) | ||||||||||||||
} else { | ||||||||||||||
Err( | ||||||||||||||
"Could not detect running starknet-devnet instance. Please use `--url <URL>` instead or start devnet if it is not running." | ||||||||||||||
.to_string(), | ||||||||||||||
) | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn find_devnet_process_info() -> Result<DevnetProcessInfo, DevnetDetectionError> { | ||||||||||||||
let output = Command::new("ps") | ||||||||||||||
.args(["aux"]) | ||||||||||||||
.output() | ||||||||||||||
.map_err(|_| DevnetDetectionError::CommandFailed)?; | ||||||||||||||
let ps_output = String::from_utf8_lossy(&output.stdout); | ||||||||||||||
|
||||||||||||||
let devnet_processes: Vec<DevnetProcessInfo> = ps_output | ||||||||||||||
.lines() | ||||||||||||||
.filter(|line| line.contains("starknet-devnet")) | ||||||||||||||
.map(|line| { | ||||||||||||||
if line.contains("docker") || line.contains("podman") { | ||||||||||||||
extract_devnet_info_from_docker_line(line) | ||||||||||||||
} else { | ||||||||||||||
extract_devnet_info_from_cmdline(line) | ||||||||||||||
} | ||||||||||||||
}) | ||||||||||||||
.collect(); | ||||||||||||||
|
||||||||||||||
match devnet_processes.as_slice() { | ||||||||||||||
[] => Err(DevnetDetectionError::NoInstance), | ||||||||||||||
[single] => Ok(single.clone()), | ||||||||||||||
_ => Err(DevnetDetectionError::MultipleInstances), | ||||||||||||||
Comment on lines
+67
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn extract_string_from_flag(cmdline: &str, flag: &str) -> Option<String> { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some helpers could go into a submodule within Generally this file could use being split into submodules. |
||||||||||||||
if let Some(pos) = cmdline.find(flag) { | ||||||||||||||
let after_pattern = &cmdline[pos + flag.len()..]; | ||||||||||||||
let value_str = after_pattern | ||||||||||||||
.split_whitespace() | ||||||||||||||
.next() | ||||||||||||||
.unwrap_or("") | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the iterator returns nothing (we unwrap with default), doesn't that mean we won't find anything anyway? Also a nit, but probably |
||||||||||||||
.trim_start_matches('=') | ||||||||||||||
.trim_start_matches(':'); | ||||||||||||||
Comment on lines
+80
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the purpose if these? |
||||||||||||||
|
||||||||||||||
if !value_str.is_empty() { | ||||||||||||||
return Some(value_str.to_string()); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
None | ||||||||||||||
} | ||||||||||||||
Comment on lines
+74
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't regex be better here? |
||||||||||||||
|
||||||||||||||
fn extract_port_from_flag(cmdline: &str, flag: &str) -> Option<u16> { | ||||||||||||||
extract_string_from_flag(cmdline, flag).and_then(|port_str| parse_valid_port(&port_str)) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn extract_docker_mapping(cmdline: &str) -> Option<(String, u16)> { | ||||||||||||||
let port_flags = ["-p", "--publish"]; | ||||||||||||||
|
||||||||||||||
for flag in &port_flags { | ||||||||||||||
if let Some(port_mapping) = extract_string_from_flag(cmdline, flag) { | ||||||||||||||
let parts: Vec<&str> = port_mapping.split(':').collect(); | ||||||||||||||
if parts.len() == 3 | ||||||||||||||
&& let Ok(host_port) = parts[1].parse::<u16>() | ||||||||||||||
{ | ||||||||||||||
return Some((parts[0].to_string(), host_port)); | ||||||||||||||
} else if parts.len() == 2 | ||||||||||||||
&& let Ok(host_port) = parts[0].parse::<u16>() | ||||||||||||||
{ | ||||||||||||||
return Some((DEFAULT_DEVNET_HOST.to_string(), host_port)); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
None | ||||||||||||||
} | ||||||||||||||
Comment on lines
+94
to
+113
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same remark about regex usage |
||||||||||||||
|
||||||||||||||
fn extract_devnet_info_from_docker_line(cmdline: &str) -> DevnetProcessInfo { | ||||||||||||||
let mut port = None; | ||||||||||||||
let mut host = None; | ||||||||||||||
|
||||||||||||||
if let Some((docker_host, docker_port)) = extract_docker_mapping(cmdline) { | ||||||||||||||
host = Some(docker_host); | ||||||||||||||
port = Some(docker_port); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
if port.is_none() { | ||||||||||||||
port = extract_port_from_flag(cmdline, "--port"); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
let final_host = host.unwrap_or_else(|| DEFAULT_DEVNET_HOST.to_string()); | ||||||||||||||
let final_port = port.unwrap_or(DEFAULT_DEVNET_PORT); | ||||||||||||||
|
||||||||||||||
DevnetProcessInfo { | ||||||||||||||
host: final_host, | ||||||||||||||
port: final_port, | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn extract_devnet_info_from_cmdline(cmdline: &str) -> DevnetProcessInfo { | ||||||||||||||
let mut port = extract_port_from_flag(cmdline, "--port"); | ||||||||||||||
let mut host = extract_string_from_flag(cmdline, "--host"); | ||||||||||||||
|
||||||||||||||
if port.is_none() { | ||||||||||||||
port = std::env::var("PORT") | ||||||||||||||
.ok() | ||||||||||||||
.and_then(|port_env| parse_valid_port(&port_env)); | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This way we silently ignore user settings if they are invalid. If we aren't doing any nice error handling for invalid values I'd rather we panic than silently ignore them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This goes for all places we do so, not only here |
||||||||||||||
} | ||||||||||||||
|
||||||||||||||
if host.is_none() | ||||||||||||||
&& let Ok(host_env) = std::env::var("HOST") | ||||||||||||||
&& !host_env.is_empty() | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We validate port but take host directly, to be consistent we should probably either validate both or none. What was the motivation for only validating one of them? |
||||||||||||||
{ | ||||||||||||||
host = Some(host_env); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
let final_port = port.unwrap_or(DEFAULT_DEVNET_PORT); | ||||||||||||||
let final_host = host.unwrap_or_else(|| DEFAULT_DEVNET_HOST.to_string()); | ||||||||||||||
|
||||||||||||||
DevnetProcessInfo { | ||||||||||||||
host: final_host, | ||||||||||||||
port: final_port, | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn is_port_reachable(host: &str, port: u16) -> bool { | ||||||||||||||
let url = format!("http://{host}:{port}/is_alive"); | ||||||||||||||
|
||||||||||||||
Command::new("curl") | ||||||||||||||
.args(["-s", "-f", "--max-time", "1", &url]) | ||||||||||||||
.output() | ||||||||||||||
.map(|output| output.status.success()) | ||||||||||||||
.unwrap_or(false) | ||||||||||||||
Comment on lines
+166
to
+170
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than rely on curl being installed, couldn't we use some library for HTTP requests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have a dependency on |
||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn parse_valid_port(port_str: &str) -> Option<u16> { | ||||||||||||||
// Ports below 1024 typically require elevated permissions | ||||||||||||||
port_str.parse::<u16>().ok().filter(|&p| p >= 1024) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
#[cfg(test)] | ||||||||||||||
mod tests { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't review the tests yet |
||||||||||||||
use super::*; | ||||||||||||||
use std::process::{Command, Stdio}; | ||||||||||||||
use std::thread; | ||||||||||||||
use std::time::{Duration, Instant}; | ||||||||||||||
|
||||||||||||||
// These tests are marked to run serially to avoid interference from environment variables | ||||||||||||||
#[test] | ||||||||||||||
fn test_devnet_parsing() { | ||||||||||||||
test_extract_devnet_info_from_cmdline(); | ||||||||||||||
|
||||||||||||||
test_extract_devnet_info_from_docker_line(); | ||||||||||||||
|
||||||||||||||
test_extract_devnet_info_with_both_envs(); | ||||||||||||||
|
||||||||||||||
test_cmdline_args_override_env(); | ||||||||||||||
|
||||||||||||||
test_detect_devnet_url(); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn test_extract_devnet_info_from_cmdline() { | ||||||||||||||
let cmdline1 = "starknet-devnet --port 6000 --host 127.0.0.1"; | ||||||||||||||
let info1 = extract_devnet_info_from_cmdline(cmdline1); | ||||||||||||||
assert_eq!(info1.port, 6000); | ||||||||||||||
assert_eq!(info1.host, "127.0.0.1"); | ||||||||||||||
|
||||||||||||||
let cmdline2 = "/usr/bin/starknet-devnet --port=5000"; | ||||||||||||||
let info2 = extract_devnet_info_from_cmdline(cmdline2); | ||||||||||||||
assert_eq!(info2.port, 5000); | ||||||||||||||
assert_eq!(info2.host, "127.0.0.1"); | ||||||||||||||
|
||||||||||||||
let cmdline3 = "starknet-devnet --host 127.0.0.1"; | ||||||||||||||
let info3 = extract_devnet_info_from_cmdline(cmdline3); | ||||||||||||||
assert_eq!(info3.port, 5050); | ||||||||||||||
assert_eq!(info3.host, "127.0.0.1"); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn test_extract_devnet_info_from_docker_line() { | ||||||||||||||
let cmdline1 = "docker run -p 127.0.0.1:5055:5050 shardlabs/starknet-devnet-rs"; | ||||||||||||||
let info1 = extract_devnet_info_from_docker_line(cmdline1); | ||||||||||||||
assert_eq!(info1.port, 5055); | ||||||||||||||
assert_eq!(info1.host, "127.0.0.1"); | ||||||||||||||
|
||||||||||||||
let cmdline2 = "docker run --publish 8080:5050 shardlabs/starknet-devnet-rs"; | ||||||||||||||
let info2 = extract_devnet_info_from_docker_line(cmdline2); | ||||||||||||||
assert_eq!(info2.port, 8080); | ||||||||||||||
assert_eq!(info2.host, "127.0.0.1"); | ||||||||||||||
|
||||||||||||||
let cmdline3 = "docker run --network host shardlabs/starknet-devnet-rs --port 5055"; | ||||||||||||||
let info3 = extract_devnet_info_from_docker_line(cmdline3); | ||||||||||||||
assert_eq!(info3.port, 5055); | ||||||||||||||
assert_eq!(info3.host, "127.0.0.1"); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn test_extract_devnet_info_with_both_envs() { | ||||||||||||||
// SAFETY: Variables are only modified within this test and cleaned up afterwards | ||||||||||||||
unsafe { | ||||||||||||||
std::env::set_var("PORT", "9999"); | ||||||||||||||
std::env::set_var("HOST", "9.9.9.9"); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
let cmdline = "starknet-devnet"; | ||||||||||||||
let info = extract_devnet_info_from_cmdline(cmdline); | ||||||||||||||
assert_eq!(info.port, 9999); | ||||||||||||||
assert_eq!(info.host, "9.9.9.9"); | ||||||||||||||
|
||||||||||||||
// SAFETY: Clean up environment variables to prevent interference | ||||||||||||||
unsafe { | ||||||||||||||
std::env::remove_var("PORT"); | ||||||||||||||
std::env::remove_var("HOST"); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn test_cmdline_args_override_env() { | ||||||||||||||
// SAFETY: Variables are only modified within this test and cleaned up afterwards | ||||||||||||||
unsafe { | ||||||||||||||
std::env::set_var("PORT", "3000"); | ||||||||||||||
std::env::set_var("HOST", "7.7.7.7"); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
let cmdline = "starknet-devnet --port 9999 --host 192.168.1.1"; | ||||||||||||||
let info = extract_devnet_info_from_cmdline(cmdline); | ||||||||||||||
assert_eq!(info.port, 9999); | ||||||||||||||
assert_eq!(info.host, "192.168.1.1"); | ||||||||||||||
|
||||||||||||||
// SAFETY: Clean up environment variables to prevent interference | ||||||||||||||
unsafe { | ||||||||||||||
std::env::remove_var("PORT"); | ||||||||||||||
std::env::remove_var("HOST"); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn test_detect_devnet_url() { | ||||||||||||||
let child = spawn_devnet("5090"); | ||||||||||||||
|
||||||||||||||
let result = detect_devnet_url().expect("Failed to detect devnet URL"); | ||||||||||||||
assert_eq!(result, "http://127.0.0.1:5090"); | ||||||||||||||
|
||||||||||||||
cleanup_process(child); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn spawn_devnet(port: &str) -> std::process::Child { | ||||||||||||||
let mut child = Command::new("starknet-devnet") | ||||||||||||||
.args(["--port", port]) | ||||||||||||||
.stdout(Stdio::null()) | ||||||||||||||
.stderr(Stdio::null()) | ||||||||||||||
.spawn() | ||||||||||||||
.expect("Failed to spawn starknet-devnet process"); | ||||||||||||||
|
||||||||||||||
let port_num: u16 = port.parse().expect("Invalid port number"); | ||||||||||||||
let start_time = Instant::now(); | ||||||||||||||
let timeout = Duration::from_secs(10); | ||||||||||||||
|
||||||||||||||
while start_time.elapsed() < timeout { | ||||||||||||||
if is_port_reachable("127.0.0.1", port_num) { | ||||||||||||||
return child; | ||||||||||||||
} | ||||||||||||||
thread::sleep(Duration::from_millis(500)); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
let _ = child.kill(); | ||||||||||||||
let _ = child.wait(); | ||||||||||||||
panic!("Devnet did not start in time on port {port}"); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fn cleanup_process(mut child: std::process::Child) { | ||||||||||||||
child.kill().expect("Failed to kill devnet process"); | ||||||||||||||
child.wait().expect("Failed to wait for devnet process"); | ||||||||||||||
} | ||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can make a separate module
devnet
, place this file and the one containingDevnetProvider
.