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.

Sign up for GitHub

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

Jump to bottom

[TECH_DEBT] Remove CommunicationChannel trait #2578

Merged
merged 12 commits into from
Feb 14, 2024
Merged

[TECH_DEBT] Remove CommunicationChannel trait #2578

merged 12 commits into from
Feb 14, 2024

Conversation

bfish713
Copy link
Collaborator

@bfish713 bfish713 commented Feb 14, 2024 •
edited
Loading

Closes #2572

This PR:

  • Gets rid of the CommunicationChannel and TestableCommunicationChannel traits
  • adjust the test generator functions such that we don't clone the libp2p layer
  • Removes so unused types/parameters for networks which we don't use (vid/viewsync) these just use the quorum network

This PR does not:

Again should not alter anything functionally as CommunicationChannel was mostly a wrapper that did nothing

Key places to review:

The updates to the 4 networking impls in crates/hotshot/src/traits/networking

@bfish713 bfish713 changed the title Bf/kill comm chan [TECH_DEBT] Remove CommunicationChannel trait Feb 14, 2024
@bfish713 bfish713 marked this pull request as ready for review February 14, 2024 16:44
rob-maron
rob-maron approved these changes Feb 14, 2024
View reviewed changes
Copy link
Collaborator

@rob-maron rob-maron left a comment

Choose a reason for hiding this comment

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

Looks great, nice work

@bfish713 bfish713 merged commit 6f7541d into main Feb 14, 2024
13 checks passed
@bfish713 bfish713 deleted the bf/kill-comm-chan branch February 14, 2024 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rob-maron rob-maron rob-maron approved these changes

@DieracDelta DieracDelta Awaiting requested review from DieracDelta

@shenkeyao shenkeyao Awaiting requested review from shenkeyao

@elliedavidson elliedavidson Awaiting requested review from elliedavidson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[TECH_DEBT] - Remove or Refactor CommunicationChannel
2 participants
@bfish713 @rob-maron