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

feat: allow StreamHanlder to broadcast StreamMessages #1253

Closed
wants to merge 1 commit into from

Conversation

guy-starkware
Copy link
Contributor

@guy-starkware guy-starkware commented Oct 8, 2024

This adds the other communications direction, e.g., from the Consensus into the network.

The StreamHandler gets a receiver of receivers (well, actually it gets a tuple with stream_id and a receiver).
Each new receiver corresponds to a new stream_id (in the Consensus case it would be a new height).

Then, messages that come in the form of generic T (can be any protobuf-able message) are packaged into StreamMessage and sent out to the network.

When the Fin message comes, the receiver channel is dropped.


This change is Reviewable

Copy link

codecov bot commented Oct 8, 2024

Codecov Report

Attention: Patch coverage is 88.23529% with 8 lines in your changes missing coverage. Please review.

Project coverage is 28.70%. Comparing base (b0cfe82) to head (4dd6edc).
Report is 302 commits behind head on main.

Files with missing lines Patch % Lines
...sequencing/papyrus_consensus/src/stream_handler.rs 89.39% 3 Missing and 4 partials ⚠️
crates/papyrus_network_types/src/network_types.rs 0.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (b0cfe82) and HEAD (4dd6edc). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (b0cfe82) HEAD (4dd6edc)
3 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1253       +/-   ##
===========================================
- Coverage   74.18%   28.70%   -45.48%     
===========================================
  Files         359       68      -291     
  Lines       36240     9077    -27163     
  Branches    36240     9077    -27163     
===========================================
- Hits        26886     2606    -24280     
+ Misses       7220     6372      -848     
+ Partials     2134       99     -2035     
Flag Coverage Δ
?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@guy-starkware guy-starkware changed the title Allow StreamHanlder to broadcast StreamMessages feat: Allow StreamHanlder to broadcast StreamMessages Oct 8, 2024
@guy-starkware guy-starkware changed the title feat: Allow StreamHanlder to broadcast StreamMessages feat: allow StreamHanlder to broadcast StreamMessages Oct 8, 2024
@guy-starkware guy-starkware force-pushed the guyn/streams/broadcast branch 3 times, most recently from f01582a to 8f20238 Compare October 14, 2024 11:41
@guy-starkware guy-starkware force-pushed the guyn/streams/broadcast branch from 8f20238 to 39a6a6f Compare October 14, 2024 11:44
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant