Skip to content
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

wip #3343

Open
wants to merge 3 commits into
base: spr/main/53a51006
Choose a base branch
from
Open

wip #3343

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion crates/papyrus_common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ pub mod pending_classes;
pub mod python_json;
pub mod state;
pub mod storage_query;
pub mod tcp;

pub(crate) fn usize_into_felt(u: usize) -> Felt {
u128::try_from(u).expect("Expect at most 128 bits").into()
Expand Down
24 changes: 0 additions & 24 deletions crates/papyrus_common/src/tcp.rs

This file was deleted.

4 changes: 4 additions & 0 deletions crates/sequencing/papyrus_consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,7 @@ test-case.workspace = true

[lints]
workspace = true

[[bin]]
name = "run_simulation"
path = "src/bin/run_simulation.rs"
12 changes: 11 additions & 1 deletion crates/sequencing/papyrus_consensus/src/bin/run_simulation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! uses the `run_consensus` binary which is able to simulate network issues for consensus messages.
use std::collections::HashSet;
use std::fs::{self, File};
use std::net::TcpListener;
use std::os::unix::process::CommandExt;
use std::process::Command;
use std::str::FromStr;
Expand All @@ -13,7 +14,6 @@ use clap::Parser;
use fs2::FileExt;
use lazy_static::lazy_static;
use nix::unistd::Pid;
use papyrus_common::tcp::find_free_port;
use papyrus_protobuf::consensus::DEFAULT_VALIDATOR_ID;
use tokio::process::Command as TokioCommand;

Expand Down Expand Up @@ -185,6 +185,16 @@ impl Drop for LockDir {
}
}

// WARNING: This is not a reliable way to obtain a free port, most notably failing when multiple
// concurrent instances rely on this mechanism.
fn find_free_port() -> u16 {
// The socket is automatically closed when the function exits.
// The port may still be available when accessed, but this is not guaranteed.
// TODO(Asmaa): find a reliable way to ensure the port stays free.
let listener = TcpListener::bind("0.0.0.0:0").expect("Failed to bind");
listener.local_addr().expect("Failed to get local address").port()
}

fn parse_duration(s: &str) -> Result<Duration, std::num::ParseIntError> {
let secs = u64::from_str(s)?;
Ok(Duration::from_secs(secs))
Expand Down
2 changes: 1 addition & 1 deletion crates/starknet_infra_utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ license-file.workspace = true
description = "Infrastructure utility."

[features]
testing = ["tokio/net"]
testing = []

[lints]
workspace = true
Expand Down
18 changes: 2 additions & 16 deletions crates/starknet_infra_utils/src/test_utils.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use std::net::{IpAddr, Ipv4Addr, SocketAddr};

use tokio::net::TcpListener;

const PORTS_PER_INSTANCE: u16 = 60;
pub const MAX_NUMBER_OF_INSTANCES_PER_TEST: u16 = 10;
const MAX_NUMBER_OF_TESTS: u16 = 10;
Expand All @@ -22,6 +20,7 @@ pub enum TestIdentifier {
EndToEndFlowTest,
MempoolSendsTxToOtherPeerTest,
MempoolReceivesTxFromOtherPeerTest,
InfraUnitTests,
}

impl From<TestIdentifier> for u16 {
Expand All @@ -31,6 +30,7 @@ impl From<TestIdentifier> for u16 {
TestIdentifier::EndToEndFlowTest => 1,
TestIdentifier::MempoolSendsTxToOtherPeerTest => 2,
TestIdentifier::MempoolReceivesTxFromOtherPeerTest => 3,
TestIdentifier::InfraUnitTests => 4,
}
}
}
Expand Down Expand Up @@ -81,17 +81,3 @@ impl AvailablePorts {
SocketAddr::new(IpAddr::from(Ipv4Addr::LOCALHOST), self.get_next_port())
}
}

/// Returns a unique IP address and port for testing purposes.
/// Tests run in parallel, so servers (like RPC or web) running on separate tests must have
/// different ports, otherwise the server will fail with "address already in use".
pub async fn get_available_socket() -> SocketAddr {
// Dynamically select port.
// First, set the port to 0 (dynamic port).
TcpListener::bind("127.0.0.1:0")
.await
.expect("Failed to bind to address")
// Then, resolve to the actual selected port.
.local_addr()
.expect("Failed to get local address")
}
1 change: 1 addition & 0 deletions crates/starknet_sequencer_infra/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ validator.workspace = true

[dev-dependencies]
assert_matches.workspace = true
once_cell.workspace = true
pretty_assertions.workspace = true
starknet-types-core.workspace = true
starknet_infra_utils = { workspace = true, features = ["testing"] }
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ use hyper::body::to_bytes;
use hyper::header::CONTENT_TYPE;
use hyper::service::{make_service_fn, service_fn};
use hyper::{Body, Client, Request, Response, Server, StatusCode, Uri};
use once_cell::sync::Lazy;
use rstest::rstest;
use serde::de::DeserializeOwned;
use serde::Serialize;
use starknet_infra_utils::test_utils::get_available_socket;
use starknet_infra_utils::test_utils::{AvailablePorts, TestIdentifier};
use starknet_types_core::felt::Felt;
use tokio::sync::mpsc::channel;
use tokio::sync::Mutex;
Expand Down Expand Up @@ -64,6 +65,12 @@ const DESERIALIZE_REQ_ERROR_MESSAGE: &str = "Could not deserialize client reques
const DESERIALIZE_RES_ERROR_MESSAGE: &str = "Could not deserialize server response";
const VALID_VALUE_A: ValueA = Felt::ONE;

// Define the shared fixture
static AVAILABLE_PORTS: Lazy<Arc<Mutex<AvailablePorts>>> = Lazy::new(|| {
let available_ports = AvailablePorts::new(TestIdentifier::InfraUnitTests.into(), 0);
Arc::new(Mutex::new(available_ports))
});

#[async_trait]
impl ComponentAClientTrait for RemoteComponentClient<ComponentARequest, ComponentAResponse> {
async fn a_get_value(&self) -> ResultA {
Expand Down Expand Up @@ -116,7 +123,7 @@ async fn create_client_and_faulty_server<T>(body: T) -> ComponentAClient
where
T: Serialize + DeserializeOwned + Debug + Send + Sync + 'static + Clone,
{
let socket = get_available_socket().await;
let socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
task::spawn(async move {
// TODO (lev, itay): Exam/change the bounds of the T for the hadler function.
async fn handler<T>(
Expand Down Expand Up @@ -193,8 +200,8 @@ async fn setup_for_tests(setup_value: ValueB, a_socket: SocketAddr, b_socket: So
#[tokio::test]
async fn test_proper_setup() {
let setup_value: ValueB = Felt::from(90);
let a_socket = get_available_socket().await;
let b_socket = get_available_socket().await;
let a_socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
let b_socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();

setup_for_tests(setup_value, a_socket, b_socket).await;
let a_client_config = RemoteClientConfig::default();
Expand All @@ -207,8 +214,8 @@ async fn test_proper_setup() {

#[tokio::test]
async fn test_faulty_client_setup() {
let a_socket = get_available_socket().await;
let b_socket = get_available_socket().await;
let a_socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
let b_socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
// Todo(uriel): Find a better way to pass expected value to the setup
// 123 is some arbitrary value, we don't check it anyway.
setup_for_tests(Felt::from(123), a_socket, b_socket).await;
Expand Down Expand Up @@ -242,7 +249,7 @@ async fn test_faulty_client_setup() {

#[tokio::test]
async fn test_unconnected_server() {
let socket = get_available_socket().await;
let socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
let client_config = RemoteClientConfig::default();
let client = ComponentAClient::new(client_config, socket);
let expected_error_contained_keywords = ["Connection refused"];
Expand Down Expand Up @@ -271,7 +278,7 @@ async fn test_faulty_server(

#[tokio::test]
async fn test_retry_request() {
let socket = get_available_socket().await;
let socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
// Spawn a server that responses with OK every other request.
task::spawn(async move {
let should_send_ok = Arc::new(Mutex::new(false));
Expand Down
Loading