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(modp2p): close peer connections after successful blocking #3838

Merged
merged 13 commits into from
Oct 18, 2024

Conversation

notlelouch
Copy link
Contributor

closes #3791

@github-actions github-actions bot added the external Issues created by non node team members label Oct 11, 2024
@cristaloleg cristaloleg changed the title Modified BlockPeer to close peer connections after successful blocking feat(modp2p): close peer connections after successful blocking Oct 11, 2024
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Thank you!

@renaynay renaynay added area:p2p kind:fix Attached to bug-fixing PRs labels Oct 11, 2024
cristaloleg
cristaloleg previously approved these changes Oct 11, 2024
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

There is a broken unit test TestP2PModule_ConnGater that's related to this change, do you mind looking into it 🙏🏻

@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.

Project coverage is 45.17%. Comparing base (2469e7a) to head (936f1a6).
Report is 342 commits behind head on main.

Files with missing lines Patch % Lines
nodebuilder/p2p/p2p.go 14.28% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3838      +/-   ##
==========================================
+ Coverage   44.83%   45.17%   +0.33%     
==========================================
  Files         265      314      +49     
  Lines       14620    21795    +7175     
==========================================
+ Hits         6555     9846    +3291     
- Misses       7313    10933    +3620     
- Partials      752     1016     +264     

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

cristaloleg
cristaloleg previously approved these changes Oct 14, 2024
@Wondertan Wondertan enabled auto-merge (squash) October 16, 2024 13:12
auto-merge was automatically disabled October 17, 2024 07:29

Head branch was pushed to by a user without write access

@cristaloleg cristaloleg enabled auto-merge (squash) October 17, 2024 08:58
@cristaloleg
Copy link
Contributor

Thanks, LGTM!

Copy link
Member

@walldiss walldiss left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you for contribution!

@Wondertan Wondertan dismissed renaynay’s stale review October 18, 2024 12:08

requested changes were fixed

@cristaloleg cristaloleg merged commit e024a67 into celestiaorg:main Oct 18, 2024
31 checks passed
Copy link

gitpoap-bot bot commented Oct 18, 2024

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2024 Celestia Contributor:

GitPOAP: 2024 Celestia Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:p2p external Issues created by non node team members kind:fix Attached to bug-fixing PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

modp2p: BlockPeer doesn't disconnect the blocked peer
7 participants