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

Store the NoiseKey in peers.rs instead of collection.rs #377

Merged
merged 6 commits into from
Mar 30, 2023

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Mar 30, 2023

Step towards #44

Right now, the NoiseKey is stored in collection.rs within an Arc. This Arc is shared with all the background tasks.

This PR removes the NoiseKey from collection.rs, and you must now pass a &NoiseKey every time you add a connection to the collection. This allows using multiple different noise keys if desired.

The NoiseKey is now stored in peers.rs. Eventually, the plan is to move it to service.rs or even outside.
Unfortunately, peers.rs might be a bit of a blocker, because the same remote peer might open multiple substreams towards the multiple local identities without it being a protocol violation.
Even if peers.rs turns out to be a blocker, the change in this PR is good even in isolation.

As part of this change, we now provide the NoiseKey immediately when initializing a single stream handshake, so that we can pass a &NoiseKey by reference at initialization rather than having to store it by value and load it later.

@tomaka
Copy link
Contributor Author

tomaka commented Mar 30, 2023

I've pushed a small side-change that replaces Box<NoiseHandshake> with NoiseHandshake.
This is because NoiseHandshake now contains a Box<NoiseHandshakeInner> rather than the fields directly.

@github-actions
Copy link

github-actions bot commented Mar 30, 2023

twiggy diff report

Difference in .wasm size before and after this pull request.


 Delta Bytes │ Item
─────────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────
       -4966 ┊ smoldot::libp2p::peers::Peers<TConn,TNow>::next_event::h02c3036cf0bd0c28
       +4966 ┊ smoldot::libp2p::peers::Peers<TConn,TNow>::next_event::h474b47a75eea3f32
       -3528 ┊ smoldot::libp2p::collection::Network<TConn,TNow>::next_event::h6273e3510799f587
       +3528 ┊ smoldot::libp2p::collection::Network<TConn,TNow>::next_event::h85ba55eb53de2f5b
       -2120 ┊ smoldot::libp2p::connection::single_stream_handshake::HealthyHandshake::read_write::ha74c0141a53f7d5b
       +2047 ┊ smoldot::libp2p::connection::single_stream_handshake::HealthyHandshake::read_write::hbdc5b44629a32efc
       -1099 ┊ smoldot::libp2p::peers::Peers<TConn,TNow>::set_peer_notifications_out_desired::h15c965098f728fa3
       +1099 ┊ smoldot::libp2p::peers::Peers<TConn,TNow>::set_peer_notifications_out_desired::h338dfb16f365001e
        -764 ┊ smoldot::libp2p::collection::Network<TConn,TNow>::new::he589a13cf9417721
        -752 ┊ smoldot::libp2p::collection::multi_stream::MultiStreamConnectionTask<TNow,TSubId>::new::heb811e910b2b79e7
        +716 ┊ smoldot::libp2p::collection::Network<TConn,TNow>::new::h131dda455042e670
        -659 ┊ smoldot::libp2p::peers::Peers<TConn,TNow>::new::h1c5e359e61382ad1
        +655 ┊ smoldot::libp2p::peers::Peers<TConn,TNow>::new::h64196d573c9d0b56
        +633 ┊ smoldot::libp2p::collection::Network<TConn,TNow>::insert_multi_stream::h7ecd2a1ab15bbdbb
        +538 ┊ smoldot::libp2p::peers::Peers<TConn,TNow>::close_out_notification::h5207b05af76e6cc0
        -538 ┊ smoldot::libp2p::peers::Peers<TConn,TNow>::close_out_notification::h52d7b2f6d9943e38
        +491 ┊ smoldot::libp2p::peers::Peers<TConn,TNow>::try_clean_up_peer::h92efec87167aa809
        -491 ┊ smoldot::libp2p::peers::Peers<TConn,TNow>::try_clean_up_peer::hcc15993d95f378a3
        -450 ┊ smoldot::libp2p::collection::Network<TConn,TNow>::insert_single_stream::h112cc92e3687c4ca
        +437 ┊ smoldot::libp2p::collection::multi_stream::MultiStreamConnectionTask<TNow,TSubId>::new::h534a70dc3c223380
        -529 ┊ ... and 173 more.
       -1127 ┊ Σ [193 Total Rows]

@tomaka tomaka enabled auto-merge March 30, 2023 14:25
@tomaka tomaka added this pull request to the merge queue Mar 30, 2023
Merged via the queue into smol-dot:main with commit 5390303 Mar 30, 2023
@tomaka tomaka deleted the rm-noise-key-req branch March 30, 2023 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant