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

refactor(iroh): remove the addr arg from start #1830

Merged
Merged
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
6 changes: 0 additions & 6 deletions iroh/src/commands/start.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ pub enum RunType {

#[derive(Args, Debug, Clone)]
pub struct StartArgs {
/// Listening address to bind to.
///
/// Only used with `start` or `--start`
#[clap(long, short, global = true, default_value_t = SocketAddr::from(iroh::node::DEFAULT_BIND_ADDR))]
addr: SocketAddr,
/// Use a token to authenticate requests for data.
///
/// Pass "random" to generate a random token, or base32-encoded bytes to use as a token
Expand Down Expand Up @@ -208,7 +203,6 @@ impl StartArgs {
.derp_mode(derp_mode)
.custom_auth_handler(Arc::new(StaticTokenAuthHandler::new(token)))
.peers_data_path(peers_data_path)
.bind_addr(self.addr)
.runtime(rt)
.rpc_endpoint(rpc_endpoint)
.secret_key(secret_key)
Expand Down
25 changes: 12 additions & 13 deletions iroh/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use std::fmt::Debug;
use std::future::Future;
use std::io;
use std::net::{Ipv4Addr, SocketAddr};
use std::net::SocketAddr;
use std::path::PathBuf;
use std::pin::Pin;
use std::sync::{Arc, Mutex};
Expand Down Expand Up @@ -76,7 +76,7 @@ const HEALTH_POLL_WAIT: Duration = Duration::from_secs(1);

/// Default bind address for the node.
/// 11204 is "iroh" in leetspeak <https://simple.wikipedia.org/wiki/Leet>
pub const DEFAULT_BIND_ADDR: (Ipv4Addr, u16) = (Ipv4Addr::LOCALHOST, 11204);
pub const DEFAULT_BIND_PORT: u16 = 11204;

/// How long we wait at most for some endpoints to be discovered.
const ENDPOINT_WAIT: Duration = Duration::from_secs(5);
Expand Down Expand Up @@ -123,7 +123,7 @@ where
S: DocStore,
E: ServiceEndpoint<ProviderService>,
{
bind_addr: SocketAddr,
bind_port: u16,
secret_key: SecretKey,
rpc_endpoint: E,
db: D,
Expand Down Expand Up @@ -169,7 +169,7 @@ impl<D: Map, S: DocStore> Builder<D, S> {
/// Creates a new builder for [`Node`] using the given database.
fn with_db_and_store(db: D, docs: S) -> Self {
Self {
bind_addr: DEFAULT_BIND_ADDR.into(),
bind_port: DEFAULT_BIND_PORT,
secret_key: SecretKey::generate(),
db,
keylog: false,
Expand Down Expand Up @@ -197,7 +197,7 @@ where
) -> Builder<D, S, E2> {
// we can't use ..self here because the return type is different
Builder {
bind_addr: self.bind_addr,
bind_port: self.bind_port,
secret_key: self.secret_key,
db: self.db,
keylog: self.keylog,
Expand Down Expand Up @@ -241,11 +241,11 @@ where
}
}

/// Binds the node service to a different socket.
/// Binds the node service to a different port.
///
/// By default it binds to `127.0.0.1:11204`.
pub fn bind_addr(mut self, addr: SocketAddr) -> Self {
self.bind_addr = addr;
pub fn bind_port(mut self, port: u16) -> Self {
self.bind_port = port;
self
}

Expand Down Expand Up @@ -318,7 +318,7 @@ where
Some(path) => endpoint.peers_data_path(path),
None => endpoint,
};
let endpoint = endpoint.bind(self.bind_addr.port()).await?;
let endpoint = endpoint.bind(self.bind_port).await?;
trace!("created quinn endpoint");

let (cb_sender, cb_receiver) = mpsc::channel(8);
Expand Down Expand Up @@ -1835,7 +1835,6 @@ impl RequestAuthorizationHandler for StaticTokenAuthHandler {
mod tests {
use anyhow::bail;
use futures::StreamExt;
use std::net::Ipv4Addr;
use std::path::Path;

use crate::rpc_protocol::WrapOption;
Expand All @@ -1857,7 +1856,7 @@ mod tests {
let doc_store = iroh_sync::store::memory::Store::default();
let hash = hashes["test"].into();
let node = Node::builder(db, doc_store)
.bind_addr((Ipv4Addr::UNSPECIFIED, 0).into())
.bind_port(0)
.runtime(&rt)
.spawn()
.await
Expand All @@ -1877,7 +1876,7 @@ mod tests {
let db = iroh_bytes::store::mem::Store::new(rt);
let doc_store = iroh_sync::store::memory::Store::default();
let node = Node::builder(db, doc_store)
.bind_addr((Ipv4Addr::UNSPECIFIED, 0).into())
.bind_port(0)
.runtime(&test_runtime())
.spawn()
.await?;
Expand All @@ -1902,7 +1901,7 @@ mod tests {
let db = iroh_bytes::store::mem::Store::new(rt);
let doc_store = iroh_sync::store::memory::Store::default();
let node = Node::builder(db, doc_store)
.bind_addr((Ipv4Addr::UNSPECIFIED, 0).into())
.bind_port(0)
.runtime(&test_runtime())
.spawn()
.await?;
Expand Down
32 changes: 5 additions & 27 deletions iroh/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use regex::Regex;
use testdir::testdir;
use walkdir::WalkDir;

const ADDR: &str = "127.0.0.1:0";
const RPC_PORT: &str = "4999";

fn make_rand_file(size: usize, path: &Path) -> Result<Hash> {
Expand Down Expand Up @@ -267,13 +266,7 @@ fn cli_provide_tree_resume() -> Result<()> {
make_rand_file(5000, &file3)?;
}
// leave the provider running for the entire test
let provider = make_provider_in(
&src_iroh_data_dir,
Input::Path(src.clone()),
false,
None,
None,
)?;
let provider = make_provider_in(&src_iroh_data_dir, Input::Path(src.clone()), false, None)?;
let count = count_input_files(&src);
let ticket = match_provide_output(&provider, count, BlobOrCollection::Collection)?;
{
Expand Down Expand Up @@ -381,8 +374,6 @@ fn cli_provide_persistence() -> anyhow::Result<()> {
iroh_bin(),
[
"start",
"--addr",
ADDR,
"--rpc-port",
"0",
"--add",
Expand Down Expand Up @@ -443,13 +434,7 @@ fn cli_provide_addresses() -> Result<()> {
make_rand_file(1000, &path)?;

let iroh_data_dir = dir.join("iroh-data-dir");
let mut provider = make_provider_in(
&iroh_data_dir,
Input::Path(path),
true,
Some("127.0.0.1:4333"),
Some(RPC_PORT),
)?;
let mut provider = make_provider_in(&iroh_data_dir, Input::Path(path), true, Some(RPC_PORT))?;
// wait for the provider to start
let _ticket = match_provide_output(&mut provider, 1, BlobOrCollection::Collection)?;

Expand Down Expand Up @@ -610,16 +595,9 @@ fn make_provider_in(
iroh_data_dir: &Path,
input: Input,
wrap: bool,
addr: Option<&str>,
rpc_port: Option<&str>,
) -> Result<ReaderHandle> {
let mut args = vec![
"start",
"--addr",
addr.unwrap_or(ADDR),
"--rpc-port",
rpc_port.unwrap_or("0"),
];
let mut args = vec!["start", "--rpc-port", rpc_port.unwrap_or("0")];
if wrap {
args.push("--wrap");
}
Expand Down Expand Up @@ -710,7 +688,7 @@ fn test_provide_get_loop(input: Input, output: Output) -> Result<()> {

let dir = testdir!();
let iroh_data_dir = dir.join("iroh-data-dir");
let mut provider = make_provider_in(&iroh_data_dir, input.clone(), wrap, None, None)?;
let mut provider = make_provider_in(&iroh_data_dir, input.clone(), wrap, None)?;

// test provide output & scrape the ticket from stderr
let ticket = match_provide_output(&mut provider, num_blobs, input.is_blob_or_collection())?;
Expand Down Expand Up @@ -776,7 +754,7 @@ fn test_provide_get_loop_single(input: Input, output: Output, hash: Hash) -> Res
let dir = testdir!();
let iroh_data_dir = dir.join("iroh-data-dir");

let mut provider = make_provider_in(&iroh_data_dir, input.clone(), true, None, None)?;
let mut provider = make_provider_in(&iroh_data_dir, input.clone(), true, None)?;

// test provide output & get all in one ticket from stderr
let ticket = match_provide_output(&mut provider, num_blobs, BlobOrCollection::Collection)?;
Expand Down
46 changes: 14 additions & 32 deletions iroh/tests/provide.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{
collections::BTreeMap,
net::{Ipv4Addr, Ipv6Addr, SocketAddr},
net::SocketAddr,
ops::Range,
path::PathBuf,
sync::Arc,
Expand Down Expand Up @@ -40,12 +40,9 @@ fn test_runtime() -> runtime::Handle {
runtime::Handle::from_current(1).unwrap()
}

fn test_node<D: Store>(
db: D,
addr: SocketAddr,
) -> Builder<D, store::memory::Store, DummyServerEndpoint> {
fn test_node<D: Store>(db: D) -> Builder<D, store::memory::Store, DummyServerEndpoint> {
let store = iroh_sync::store::memory::Store::default();
Node::builder(db, store).bind_addr(addr)
Node::builder(db, store).bind_port(0)
}

#[tokio::test]
Expand Down Expand Up @@ -147,7 +144,6 @@ fn get_options(node_id: NodeId, addrs: Vec<SocketAddr>) -> iroh::dial::Options {
#[tokio::test(flavor = "multi_thread")]
async fn multiple_clients() -> Result<()> {
let content = b"hello world!";
let addr = "127.0.0.1:0".parse().unwrap();

let mut db = iroh_bytes::store::readonly_mem::Store::default();
let expect_hash = db.insert(content.as_slice());
Expand All @@ -161,7 +157,7 @@ async fn multiple_clients() -> Result<()> {
)?;
let hash = db.insert_many(collection.to_blobs()).unwrap();
let rt = test_runtime();
let node = test_node(db, addr).runtime(&rt).spawn().await?;
let node = test_node(db).runtime(&rt).spawn().await?;

let mut tasks = Vec::new();
for _i in 0..3 {
Expand Down Expand Up @@ -250,8 +246,7 @@ where
// sort expects by name to match the canonical order of blobs
expects.sort_by(|a, b| a.0.cmp(&b.0));

let addr = "127.0.0.1:0".parse().unwrap();
let node = test_node(mdb.clone(), addr).runtime(rt).spawn().await?;
let node = test_node(mdb.clone()).runtime(rt).spawn().await?;

let (events_sender, mut events_recv) = mpsc::unbounded_channel();

Expand Down Expand Up @@ -356,8 +351,7 @@ async fn test_server_close() {
)
.unwrap();
let hash = db.insert_many(collection.to_blobs()).unwrap();
let addr = "127.0.0.1:0".parse().unwrap();
let mut node = test_node(db, addr).runtime(&rt).spawn().await.unwrap();
let mut node = test_node(db).runtime(&rt).spawn().await.unwrap();
let node_addr = node.local_endpoint_addresses().await.unwrap();
let peer_id = node.node_id();

Expand Down Expand Up @@ -429,8 +423,7 @@ async fn test_ipv6() {
let rt = test_runtime();

let (db, hash) = create_test_db([("test", b"hello")]);
let addr = (Ipv6Addr::UNSPECIFIED, 0).into();
let node = match test_node(db, addr).runtime(&rt).spawn().await {
let node = match test_node(db).runtime(&rt).spawn().await {
Ok(provider) => provider,
Err(_) => {
// We assume the problem here is IPv6 on this host. If the problem is
Expand Down Expand Up @@ -458,8 +451,7 @@ async fn test_not_found() {

let db = iroh_bytes::store::readonly_mem::Store::default();
let hash = blake3::hash(b"hello").into();
let addr = (Ipv6Addr::UNSPECIFIED, 0).into();
let node = match test_node(db, addr).runtime(&rt).spawn().await {
let node = match test_node(db).runtime(&rt).spawn().await {
Ok(provider) => provider,
Err(_) => {
// We assume the problem here is IPv6 on this host. If the problem is
Expand Down Expand Up @@ -502,8 +494,7 @@ async fn test_chunk_not_found_1() {
let data = (0..1024 * 64).map(|i| i as u8).collect::<Vec<_>>();
let hash = blake3::hash(&data).into();
let _entry = db.get_or_create_partial(hash, data.len() as u64).unwrap();
let addr = (Ipv6Addr::UNSPECIFIED, 0).into();
let node = match test_node(db, addr).runtime(&rt).spawn().await {
let node = match test_node(db).runtime(&rt).spawn().await {
Ok(provider) => provider,
Err(_) => {
// We assume the problem here is IPv6 on this host. If the problem is
Expand Down Expand Up @@ -541,8 +532,7 @@ async fn test_run_ticket() {
let rt = test_runtime();
let (db, hash) = create_test_db([("test", b"hello")]);
let token = Some(RequestToken::generate());
let addr = (Ipv4Addr::UNSPECIFIED, 0).into();
let node = test_node(db, addr)
let node = test_node(db)
.custom_auth_handler(Arc::new(StaticTokenAuthHandler::new(token.clone())))
.runtime(&rt)
.spawn()
Expand Down Expand Up @@ -615,8 +605,7 @@ async fn run_collection_get_request(
async fn test_run_fsm() {
let rt = test_runtime();
let (db, hash) = create_test_db([("a", b"hello"), ("b", b"world")]);
let addr = (Ipv4Addr::UNSPECIFIED, 0).into();
let node = test_node(db, addr).runtime(&rt).spawn().await.unwrap();
let node = test_node(db).runtime(&rt).spawn().await.unwrap();
let addrs = node.local_endpoint_addresses().await.unwrap();
let peer_id = node.node_id();
tokio::time::timeout(Duration::from_secs(10), async move {
Expand Down Expand Up @@ -665,8 +654,7 @@ async fn test_size_request_blob() {
let last_chunk = last_chunk(&expected);
let (db, hashes) = iroh_bytes::store::readonly_mem::Store::new([("test", &expected)]);
let hash = Hash::from(*hashes.values().next().unwrap());
let addr = "127.0.0.1:0".parse().unwrap();
let node = test_node(db, addr).runtime(&rt).spawn().await.unwrap();
let node = test_node(db).runtime(&rt).spawn().await.unwrap();
let addrs = node.local_endpoint_addresses().await.unwrap();
let peer_id = node.node_id();
tokio::time::timeout(Duration::from_secs(10), async move {
Expand All @@ -693,12 +681,7 @@ async fn test_collection_stat() {
let child1 = make_test_data(123456);
let child2 = make_test_data(345678);
let (db, hash) = create_test_db([("a", &child1), ("b", &child2)]);
let addr = "127.0.0.1:0".parse().unwrap();
let node = test_node(db.clone(), addr)
.runtime(&rt)
.spawn()
.await
.unwrap();
let node = test_node(db.clone()).runtime(&rt).spawn().await.unwrap();
let addrs = node.local_endpoint_addresses().await.unwrap();
let peer_id = node.node_id();
tokio::time::timeout(Duration::from_secs(10), async move {
Expand Down Expand Up @@ -759,8 +742,7 @@ async fn test_token_passthrough() -> Result<()> {
let rt = test_runtime();
let expected = b"hello".to_vec();
let (db, hash) = create_test_db([("test", expected.clone())]);
let addr = SocketAddr::from((Ipv4Addr::LOCALHOST, 0));
let node = test_node(db, addr)
let node = test_node(db)
.custom_auth_handler(Arc::new(CustomAuthHandler))
.runtime(&rt)
.spawn()
Expand Down
7 changes: 1 addition & 6 deletions iroh/tests/sync.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::{
future::Future,
net::SocketAddr,
sync::Arc,
time::{Duration, Instant},
};
Expand Down Expand Up @@ -37,7 +36,6 @@ fn test_runtime() -> runtime::Handle {

fn test_node(
rt: runtime::Handle,
addr: SocketAddr,
secret_key: SecretKey,
) -> Builder<iroh_bytes::store::mem::Store, store::memory::Store, DummyServerEndpoint> {
let db = iroh_bytes::store::mem::Store::new(rt.clone());
Expand All @@ -46,7 +44,6 @@ fn test_node(
.secret_key(secret_key)
.derp_mode(DerpMode::Disabled)
.runtime(&rt)
.bind_addr(addr)
}

// The function is not `async fn` so that we can take a `&mut` borrow on the `rng` without
Expand All @@ -59,7 +56,7 @@ fn spawn_node(
) -> impl Future<Output = anyhow::Result<Node<iroh_bytes::store::mem::Store>>> + 'static {
let secret_key = SecretKey::generate_with_rng(rng);
async move {
let node = test_node(rt, "127.0.0.1:0".parse()?, secret_key);
let node = test_node(rt, secret_key);
let node = node.spawn().await?;
info!(?i, me = %node.node_id().fmt_short(), "node spawned");
Ok(node)
Expand Down Expand Up @@ -738,11 +735,9 @@ async fn doc_delete() -> Result<()> {
let rt = test_runtime();
let db = iroh_bytes::store::mem::Store::new(rt.clone());
let store = iroh_sync::store::memory::Store::default();
let addr = "127.0.0.1:0".parse().unwrap();
let node = Node::builder(db, store)
.gc_policy(iroh::node::GcPolicy::Interval(Duration::from_millis(100)))
.runtime(&rt)
.bind_addr(addr)
.spawn()
.await?;
let client = node.client();
Expand Down
Loading