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

fix default filtered p2p ip list (non routable ips that should not be announced) #699

Merged
merged 3 commits into from
Sep 30, 2024

Conversation

paulo-ocean
Copy link
Contributor

Fixes #691 .

Changes proposed in this PR:

  • set proper default non public ips on P2P_FILTER_ANNOUNCED_ADDRESSES

@paulo-ocean
Copy link
Contributor Author

need to fix tests, break unit tests with 2 local nodes

@paulo-ocean paulo-ocean self-assigned this Sep 26, 2024
@paulo-ocean paulo-ocean marked this pull request as ready for review September 26, 2024 09:59
Copy link
Member

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

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

Although I do not fully support this because of two reasons:

  1. users should learn networking, otherwise they will not understand what's happening
  2. this breaks complex setups, like private networks of nodes or semi private (a couple of nodes inside an interna network that are using some relay servers to protect them selves from outside)

I will approve it just because it makes life eaiser for noobies .

One small suggestion and we are good to go

@@ -55,6 +56,7 @@ describe('OceanP2P Test', () => {
config2.p2pConfig.bootstrapNodes = []
// enable private IP
config2.p2pConfig.announcePrivateIp = true
config1.p2pConfig.filterAnnouncedAddresses = ['172.15.0.0/24'] // allow nodes to see each other locally for tests
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't be config2 ?

Suggested change
config1.p2pConfig.filterAnnouncedAddresses = ['172.15.0.0/24'] // allow nodes to see each other locally for tests
config2.p2pConfig.filterAnnouncedAddresses = ['172.15.0.0/24'] // allow nodes to see each other locally for tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah :-) .... damn copy/paste

@alexcos20 alexcos20 merged commit 50c4bcb into main Sep 30, 2024
12 checks passed
@alexcos20 alexcos20 deleted the issue-691-p2p-ip-range branch September 30, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

correct the default value for P2P_FILTER_ANNOUNCED_ADDRESSES
2 participants