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

p2p: HTTP catchup over p2p network #5898

Merged
merged 16 commits into from
Jan 29, 2024

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Jan 9, 2024

Summary

  • Upgraded go-libp2p to v0.32.2 in order to get p2p http feature. This pulled quite a bit of dependences updates with the following broken (fixes not related to the PR topic):
    • slices.SortFunc interface change
    • capabilities test failures
  • libp2p http does not support path parameter we use in services, so p2pNetwork uses own intermediate gorilla/mux router and registers self as a root (/) handler.
  • In order to let the universalFetcher to distinguish between p2p and regular tcp peers, peer's rootURL checked if it is a multiaddr
  • DHT is only queried on demand as peerSelector requires. The BackoffDiscovery::FindPeers has a cache so we are not making crazy number or requests to DHT.

Test Plan

Added a unit test to ./node to test http catchup via p2p transport.
Reenabled some skipped tests in node_test.go

@algorandskiy algorandskiy self-assigned this Jan 9, 2024
@algorandskiy algorandskiy changed the title tests: fixes to node_test p2p tests: fixes to node_test Jan 9, 2024
Copy link

codecov bot commented Jan 9, 2024

Codecov Report

Attention: 87 lines in your changes are missing coverage. Please review.

Comparison is base (63d4995) 55.91% compared to head (17a8fe8) 56.04%.

Files Patch % Lines
network/p2pNetwork.go 60.19% 35 Missing and 6 partials ⚠️
network/p2p/streams.go 0.00% 27 Missing ⚠️
network/p2p/http.go 0.00% 9 Missing ⚠️
catchup/universalFetcher.go 44.44% 2 Missing and 3 partials ⚠️
data/transactions/logic/opcodes.go 50.00% 1 Missing and 2 partials ⚠️
network/p2p/peerID.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           feature/p2p    #5898      +/-   ##
===============================================
+ Coverage        55.91%   56.04%   +0.12%     
===============================================
  Files              481      482       +1     
  Lines            67938    68078     +140     
===============================================
+ Hits             37987    38153     +166     
+ Misses           27375    27326      -49     
- Partials          2576     2599      +23     

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

@algorandskiy algorandskiy changed the title p2p tests: fixes to node_test p2p: HTTP catchup over p2p network Jan 26, 2024
@algorandskiy algorandskiy marked this pull request as ready for review January 26, 2024 17:34
@algorandskiy algorandskiy added the p2p Work related to the p2p project label Jan 26, 2024
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

I don't know a lot about libp2p HTTP handling, but this looks good. I've left some spelling corrections and a test suggestion, but those are minor

@algorandskiy
Copy link
Contributor Author

algorandskiy commented Jan 29, 2024

Thank you Jason, I'll include these fixes into a followup PR I already started.

@algorandskiy algorandskiy merged commit d69938a into algorand:feature/p2p Jan 29, 2024
16 checks passed
@algorandskiy
Copy link
Contributor Author

Applied in #5922

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.

2 participants