feat(network): register_broadcast_subscriber receives any type with TryFrom/Into<Bytes>

[ShahakShama]

• refactor(network): mixed event contains the outputting behaviour instead of notified behaviour
• feat(network): add broadcast behaviour with unimplemented
• feat(network): connect broadcast behaviour to mixed behaviour
• feat(network): add register_broadcast_subscribers without connecting to behaviour
• refactor(network): move code
• refactor(network): make StreamHashMap usable in source code (not tests only)
• feat(network): connect broadcast subscribers to behaviour
• test(network): add tests for broadcast subscriber in network manager
• fix(network): report callback reports the originating peer
• feat(network): register_broadcast_subscriber receives any type with TryFrom/Into

---

This change is 

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.16%. Comparing base (e15a3aa) to head (549cd1f).
Report is 22 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2001      +/-   ##
==========================================
- Coverage   71.60%   70.16%   -1.45%     
==========================================
  Files         129      131       +2     
  Lines       16751    17092     +341     
  Branches    16751    17092     +341     
==========================================
- Hits        11995    11992       -3     
- Misses       3417     3774     +357     
+ Partials     1339     1326      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[matan-starkware]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dan-starkware and @ShahakShama)

---

crates/papyrus_network/src/network_manager/mod.rs line 114 at r1 (raw file):

    where
        T: TryFrom<Bytes>,
        Bytes: From<T>,

So this means that we handle deserialization errors T: TryFrom<Bytes> but we expect serialization to be infallible Bytes: From<T>?

Code quote:

        T: TryFrom<Bytes>,
        Bytes: From<T>,

---

crates/papyrus_network/src/network_manager/mod.rs line 494 at r1 (raw file):

pub type ReportCallback = Box<dyn Fn() + Send>;

// TODO(shahak): Make this generic in a type that implements TryFrom<Bytes> and Into<Bytes>.

Can this be removed? Do we not need to specify here Into/TryFrom on T?

Code quote:

// TODO(shahak): Make this generic in a type that implements TryFrom<Bytes> and Into<Bytes>.

---

crates/papyrus_network/src/network_manager/mod.rs line 506 at r1 (raw file):

        Receiver<(Bytes, ReportCallback)>,
        fn((Bytes, ReportCallback)) -> (Result<T, E>, ReportCallback),
    >,

This is quite ugly. Maybe we should have a comment explaining the types?

Code quote:

pub struct BroadcastSubscriberChannels<T, E> {
    pub messages_to_broadcast_sender: With<
        Sender<Bytes>,
        Bytes,
        T,
        Ready<Result<Bytes, SendError>>,
        fn(T) -> Ready<Result<Bytes, SendError>>,
    >,
    pub broadcasted_messages_receiver: Map<
        Receiver<(Bytes, ReportCallback)>,
        fn((Bytes, ReportCallback)) -> (Result<T, E>, ReportCallback),
    >,

---

crates/papyrus_network/src/network_manager/test.rs line 482 at r1 (raw file):

    let mut broadcasted_messages_receiver = network_manager
        .register_broadcast_subscriber::<Bytes>(topic.clone(), BUFFER_SIZE)

So does this mean that we are converting the incoming Vec<u8> into Bytes? Should we try something with actual deserialization?

Code quote:

::<Bytes>

[ShahakShama]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dan-starkware and @matan-starkware)

---

crates/papyrus_network/src/network_manager/mod.rs line 114 at r1 (raw file):

Previously, matan-starkware wrote…

So this means that we handle deserialization errors T: TryFrom<Bytes> but we expect serialization to be infallible Bytes: From<T>?

Yes

---

crates/papyrus_network/src/network_manager/mod.rs line 494 at r1 (raw file):

Previously, matan-starkware wrote…

Can this be removed? Do we not need to specify here Into/TryFrom on T?

Done.

---

crates/papyrus_network/src/network_manager/mod.rs line 506 at r1 (raw file):

Previously, matan-starkware wrote…

This is quite ugly. Maybe we should have a comment explaining the types?

clippy agrees with you 😅
Is now better?

---

crates/papyrus_network/src/network_manager/test.rs line 482 at r1 (raw file):

Previously, matan-starkware wrote…

So does this mean that we are converting the incoming Vec<u8> into Bytes? Should we try something with actual deserialization?

Yes. In a future PR I did an e2e test for the broadcast and there I putted a "real" deserialization

[matan-starkware]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dan-starkware and @ShahakShama)

---

crates/papyrus_network/src/network_manager/mod.rs line 506 at r1 (raw file):

Previously, ShahakShama wrote…

clippy agrees with you 😅
Is now better?

Matan Clippy Markind

It's better. It still feels like there is a lot of type complexity, but I also feel like reducing that would mean a significant change in approach, which I am not asking for.