Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

default ws note #10014

Closed
wants to merge 12 commits into from
2 changes: 1 addition & 1 deletion bin/node/cli/benches/transaction_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase {
rpc_http: None,
rpc_ws: None,
rpc_ipc: None,
rpc_ws_max_connections: None,
rpc_ws_max_connections: usize::MAX,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, please don't use usize::MAX

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be a sensible alternative? Use usize::MIN or rely on the Rust language default?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will 1024 be good?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual default is 100 as implied by the default value for the CLI option.

Regarding the review comment above, we are basically looking for something to use instead of None because we no longer use an Option<>.

rpc_cors: None,
rpc_methods: Default::default(),
rpc_max_payload: None,
Expand Down
21 changes: 10 additions & 11 deletions client/cli/src/commands/run_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,8 @@ pub struct RunCmd {
/// Maximum number of WS RPC server connections.
///
/// Default is 100.
// {this is set in client/rpc-servers/src/lib.rs}
#[structopt(long = "ws-max-connections", value_name = "COUNT")]
pub ws_max_connections: Option<usize>,
#[structopt(long = "ws-max-connections", value_name = "COUNT", default_value = "100")]
pub ws_max_connections: usize,

/// Specify browser Origins allowed to access the HTTP & WS RPC servers.
///
Expand Down Expand Up @@ -378,7 +377,7 @@ impl CliConfiguration for RunCmd {
Ok(self.no_grandpa)
}

fn rpc_ws_max_connections(&self) -> Result<Option<usize>> {
fn rpc_ws_max_connections(&self) -> Result<usize> {
Ok(self.ws_max_connections)
}

Expand Down Expand Up @@ -463,19 +462,19 @@ impl CliConfiguration for RunCmd {
pub fn is_node_name_valid(_name: &str) -> std::result::Result<(), &str> {
let name = _name.to_string();
if name.chars().count() >= crate::NODE_NAME_MAX_LENGTH {
return Err("Node name too long")
return Err("Node name too long");
}

let invalid_chars = r"[\\.@]";
let re = Regex::new(invalid_chars).unwrap();
if re.is_match(&name) {
return Err("Node name should not contain invalid chars such as '.' and '@'")
return Err("Node name should not contain invalid chars such as '.' and '@'");
}

let invalid_patterns = r"(https?:\\/+)?(www)+";
let re = Regex::new(invalid_patterns).unwrap();
if re.is_match(&name) {
return Err("Node name should not contain urls")
return Err("Node name should not contain urls");
}

Ok(())
Expand All @@ -493,7 +492,7 @@ fn rpc_interface(
a validator. Use `--unsafe-rpc-external` or `--rpc-methods=unsafe` if you understand \
the risks. See the options description for more information."
.to_owned(),
))
));
}

if is_external || is_unsafe_external {
Expand Down Expand Up @@ -536,7 +535,7 @@ fn parse_telemetry_endpoints(s: &str) -> std::result::Result<(String, u8), Telem
let verbosity =
s[pos_ + 1..].parse().map_err(TelemetryParsingError::VerbosityParsingError)?;
Ok((url, verbosity))
},
}
}
}

Expand Down Expand Up @@ -569,8 +568,8 @@ fn parse_cors(s: &str) -> std::result::Result<Cors, Box<dyn std::error::Error>>
match part {
"all" | "*" => {
is_all = true;
break
},
break;
}
other => origins.push(other.to_owned()),
}
}
Expand Down
6 changes: 3 additions & 3 deletions client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,8 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
/// Get the RPC websockets maximum connections (`None` if unlimited).
///
/// By default this is `None`.
fn rpc_ws_max_connections(&self) -> Result<Option<usize>> {
Ok(None)
fn rpc_ws_max_connections(&self) -> Result<usize> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs above are incorrect.

Ok(usize::MAX)
}

/// Get the RPC cors (`None` if disabled)
Expand Down Expand Up @@ -598,7 +598,7 @@ pub fn generate_node_name() -> String {
let count = node_name.chars().count();

if count < NODE_NAME_MAX_LENGTH {
return node_name
return node_name;
}
}
}
9 changes: 3 additions & 6 deletions client/rpc-servers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ const MEGABYTE: usize = 1024 * 1024;
/// Maximal payload accepted by RPC servers.
pub const RPC_MAX_PAYLOAD_DEFAULT: usize = 15 * MEGABYTE;

/// Default maximum number of connections for WS RPC servers.
const WS_MAX_CONNECTIONS: usize = 100;

/// The RPC IoHandler containing all requested APIs.
pub type RpcHandler<T> = pubsub::PubSubHandler<T, RpcMiddleware>;

Expand Down Expand Up @@ -168,7 +165,7 @@ pub fn start_ws<
M: pubsub::PubSubMetadata + From<futures::channel::mpsc::UnboundedSender<String>>,
>(
addr: &std::net::SocketAddr,
max_connections: Option<usize>,
max_connections: usize,
cors: Option<&Vec<String>>,
io: RpcHandler<M>,
maybe_max_payload_mb: Option<usize>,
Expand All @@ -183,7 +180,7 @@ pub fn start_ws<
})
.event_loop_executor(tokio_handle)
.max_payload(rpc_max_payload)
.max_connections(max_connections.unwrap_or(WS_MAX_CONNECTIONS))
.max_connections(max_connections)
.allowed_origins(map_cors(cors))
.allowed_hosts(hosts_filtering(cors.is_some()))
.session_stats(server_metrics)
Expand All @@ -194,7 +191,7 @@ pub fn start_ws<
e => {
error!("{}", e);
io::ErrorKind::Other.into()
},
}
})
}

Expand Down
6 changes: 3 additions & 3 deletions client/service/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ pub struct Configuration {
pub rpc_ws: Option<SocketAddr>,
/// RPC over IPC binding path. `None` if disabled.
pub rpc_ipc: Option<String>,
/// Maximum number of connections for WebSockets RPC server. `None` if default.
pub rpc_ws_max_connections: Option<usize>,
/// Maximum number of connections for WebSockets RPC server.
pub rpc_ws_max_connections: usize,
/// CORS settings for HTTP & WS servers. `None` if all origins are allowed.
pub rpc_cors: Option<Vec<String>>,
/// RPC methods to expose (by default only a safe subset or all of them).
Expand Down Expand Up @@ -217,7 +217,7 @@ impl Configuration {
crate::DEFAULT_PROTOCOL_ID
);
crate::DEFAULT_PROTOCOL_ID
},
}
};
sc_network::config::ProtocolId::from(protocol_id_full)
}
Expand Down
2 changes: 1 addition & 1 deletion client/service/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ fn node_config<
rpc_http: None,
rpc_ipc: None,
rpc_ws: None,
rpc_ws_max_connections: None,
rpc_ws_max_connections: usize::MAX,
rpc_cors: None,
rpc_methods: Default::default(),
rpc_max_payload: None,
Expand Down
2 changes: 1 addition & 1 deletion test-utils/test-runner/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub fn default_config(tokio_handle: Handle, mut chain_spec: Box<dyn ChainSpec>)
rpc_http: None,
rpc_ws: None,
rpc_ipc: None,
rpc_ws_max_connections: None,
rpc_ws_max_connections: usize::MAX,
rpc_cors: None,
rpc_methods: Default::default(),
rpc_max_payload: None,
Expand Down