-
Notifications
You must be signed in to change notification settings - Fork 56
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
feat: client webtransport-websys feat #1224
Conversation
69794b5
to
2d93509
Compare
assert_eq!(format!("/ip4/127.0.0.1/tcp/11101/p2p/{peer_id}"), multiaddr); | ||
if !cfg!(feature = "websockets") { | ||
assert_eq!( | ||
format!("/ip4/127.0.0.1/tcp/11101/ws/p2p/{peer_id}"), |
Check notice
Code scanning / devskim
Accessing localhost could indicate debug code, or could hinder scaling. Note
assert_eq!(format!("/ip4/127.0.0.1/tcp/11101/p2p/{peer_id}"), multiaddr); | ||
if cfg!(feature = "websockets") { | ||
assert_eq!( | ||
format!("/ip4/127.0.0.1/tcp/11101/ws/p2p/{peer_id}"), |
Check notice
Code scanning / devskim
Accessing localhost could indicate debug code, or could hinder scaling. Note
let genesis_multi_addr = if cfg!(any(feature = "websockets", target_arch = "wasm32")) { | ||
format!("/ip4/127.0.0.1/tcp/{genesis_port:?}/ws/p2p/{peer_id}") | ||
} else { | ||
format!("/ip4/127.0.0.1/udp/{genesis_port:?}/quic-v1/p2p/{peer_id}") |
Check notice
Code scanning / devskim
Accessing localhost could indicate debug code, or could hinder scaling. Note
format!("/ip4/127.0.0.1/udp/{genesis_port:?}/quic-v1/p2p/{peer_id}"); | ||
|
||
let genesis_multi_addr = if cfg!(any(feature = "websockets", target_arch = "wasm32")) { | ||
format!("/ip4/127.0.0.1/tcp/{genesis_port:?}/ws/p2p/{peer_id}") |
Check notice
Code scanning / devskim
Accessing localhost could indicate debug code, or could hinder scaling. Note
2d93509
to
63bae00
Compare
))); | ||
|
||
|
||
// TODO: We need to tidy this up, the client loops forever in the browser, and eventually crashes |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
console::log_1(&JsValue::from_str("Hello safe world!")); | ||
|
||
// Tracing | ||
// TODO: dont log _everything_ |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
0896de4
to
2ca735f
Compare
#[cfg(feature = "tcp")] | ||
let main_transport = TokioTransport::new(TransportConfig::default()) | ||
#[cfg(target_arch = "wasm32")] | ||
let main_transport = WebSocketTransport::default() |
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.
same idea as before, move to separate file and create a create_main_transport
function
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.
I did look at this, but things got unwieldy fast.
I'm not against it, but I don't think it's a blocker for this PR.
@@ -87,6 +95,8 @@ pub const fn close_group_majority() -> usize { | |||
|
|||
/// Max duration for all GET attempts | |||
const MAX_GET_RETRY_DURATION_MS: u64 = 6800; | |||
|
|||
#[cfg(not(target_arch = "wasm32"))] |
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.
wasm doesn't need MAX_GET_RETRY_DURATION
?
I thought this is a non-transport-dependent retry config ?
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 cannot (at the moment) easily have backoff with wasm32. backoff uses tokio right now.
We can probably rejig this, but i'd say that's an optimisation atm. The main aim here (i think) is to get the wasm code building and checked?
e7cdd2a
to
9d8c040
Compare
9d8c040
to
22cc892
Compare
3776108
to
aacb678
Compare
Description
Summary generated by Reviewpad on 24 Jan 24 15:33 UTC
This pull request includes multiple file diffs. Here is a summarized overview of the changes:
The
cmd.rs
file in thesn_networking
module has several modifications, including new imports, changes in enum variants and struct methods, variable updates, and code formatting changes.The
replication_fetcher.rs
file has changes related to imports, removing and adding features, and updating comments and code related to parallel fetches.The
Cargo.toml
file removes the "quic" feature and includes only the "metrics" feature.The
wallet.rs
file has changes in imports related to time and duration, along with a comment explaining the usage of a wallet client for token transfers.The
lib.rs
file has modifications in thegenesis_multi_addr
variable and related assertions in tests.The README.md file has updates regarding transport protocols, architectures, and building for
wasm32
.The diff adds a new
target_arch.rs
file with conditionally imported modules based on the target architecture. Changes also include importingspawn_local
for handling asynchronous tasks.The
lib.rs
file includes changes in importing theDuration
struct from thetokio::time
module, replacing the previous import from thestd::time
module.The diff for the file
download.rs
includes import changes related to time, removing and replacing import statements.The
Cargo.toml
file has changes in dependency versions, feature modifications, and wasm compilation options.The file
client.rs
intests/common
has changes in imports related to time.The
upload.rs
file has modifications related to logging and changing the method of reading chunk bytes.The diff includes changes in importing the
sleep
function and related replacements for time-related functions.The
Cargo.toml
file includes changes in features, dependencies, and versions.The
driver.rs
file within thesn_networking
module has various changes, including conditional compilation, transport updates, logging, and configurations based on build and target architectures.The
node_registry.rs
file has changes in constructing theaddr
variable based on feature flags for different build configurations.The diff introduces modifications related to imports, including the addition and removal of import statements for various modules, types, and libraries.
The
Cargo.toml
file includes changes in dependencies, removing and adding features.The diff includes changes in importing modules, types, and constants in the file.
The
Cargo.toml
file has changes in dependencies, including the removal of the "default" and "quic" features.Please let me know if you need more details or specific information for any of the file diffs.