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: Return non-loopback multiaddr from Address() if available #5716

Merged
merged 2 commits into from
Sep 1, 2023

Conversation

iansuvak
Copy link
Contributor

Summary

When fetching data about the node from the p2p.Service we might end up with a loopback address /ip4/127.0.0.1/tcp/<port> This works fine for local tests but not for functionality depending on this value spanning multiple nodes.

This change fetches the first non-loopback address that's available and if none is available reverts to the old functionality of returning the first one.

Test Plan

Added a new test for the functionality
Also added a mockService struct that we can use from the network if we need to test other functionality

@iansuvak iansuvak added Enhancement Team Carbon-11 p2p Work related to the p2p project labels Aug 30, 2023
@iansuvak iansuvak requested a review from cce August 30, 2023 17:17
@codecov
Copy link

codecov bot commented Aug 30, 2023

Codecov Report

Merging #5716 (d81a652) into master (2670e94) will decrease coverage by 0.01%.
Report is 5 commits behind head on master.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #5716      +/-   ##
==========================================
- Coverage   55.51%   55.51%   -0.01%     
==========================================
  Files         473      473              
  Lines       66316    66320       +4     
==========================================
+ Hits        36814    36816       +2     
- Misses      27005    27008       +3     
+ Partials     2497     2496       -1     
Files Changed Coverage Δ
network/p2pNetwork.go 67.11% <88.88%> (+5.19%) ⬆️

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

cce
cce previously approved these changes Aug 31, 2023
network/p2pNetwork_test.go Outdated Show resolved Hide resolved
@cce cce merged commit 3c77a7f into algorand:master Sep 1, 2023
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 Team Carbon-11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants