Skip to content

Conversation

@algorandskiy
Copy link
Contributor

Summary

Added EnableGossipService support to p2p net: stream manager checks connection direction and either does not open a new /algorand-ws/1.0.0 (in Connected notification) or closes incoming stream in a stream handler.

Additionally I removed useless incoming vs outgoing checks with warnings since their assumption are wrong: Connected notification is fired for both incoming and outgoing connections.

Test Plan

Added tests to ensure:

  1. Node with EnableGossipService=false can still accept and send traffic
  2. Relay and node with EnableGossipService=false cannot communicate.

@algorandskiy algorandskiy added Enhancement p2p Work related to the p2p project labels Jul 19, 2024
@algorandskiy algorandskiy self-assigned this Jul 19, 2024
@codecov
Copy link

codecov bot commented Jul 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 56.30%. Comparing base (edda2ee) to head (4673c8b).

Files Patch % Lines
network/p2p/streams.go 0.00% 14 Missing ⚠️
network/p2p/p2p.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6073      +/-   ##
==========================================
+ Coverage   55.95%   56.30%   +0.34%     
==========================================
  Files         488      488              
  Lines       69602    69595       -7     
==========================================
+ Hits        38945    39182     +237     
+ Misses      27967    27763     -204     
+ Partials     2690     2650      -40     

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

Copy link
Contributor

@gmalouf gmalouf left a comment

Choose a reason for hiding this comment

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

One still is receiving blocks and certs when gossip service is disabled right? Do we doublecheck that/is it worth doublechecking that?

gmalouf
gmalouf previously approved these changes Jul 19, 2024
jasonpaulos
jasonpaulos previously approved these changes Jul 25, 2024
@algorandskiy
Copy link
Contributor Author

Merged master into

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement p2p Work related to the p2p project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants