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

add ipv6 listening addresses to the default config #1010

Merged
merged 12 commits into from
Feb 28, 2020

Conversation

hsanjuan
Copy link
Collaborator

Taking over from #1002

@RubenKelevra

It is not supported by docker by default. It is not supported in travis-CI
build environments. User can enable it now manually.
@hsanjuan
Copy link
Collaborator Author

@RubenKelevra travis ci, and docker don't have ipv6 enabled by default. I have disabled it again. I don't want peers failing to start on default configurations because the platform does not happen to be configured for ipv6.

@RubenKelevra
Copy link
Collaborator

RubenKelevra commented Feb 21, 2020

@RubenKelevra travis ci, and docker don't have ipv6 enabled by default. I have disabled it again. I don't want peers failing to start on default configurations because the platform does not happen to be configured for ipv6.

Right, valid points.

How about removing the explicit listening address configuration from the default config completely, like many daemons do (sshd for example).

If the user choose to explicitly configure an IPv6 or IPv4 listening address in the configuration, we can still fatally fail, since this is clearly a missing precondition.

If there's no configuration for listening addresses we just attach to ::1/127.0.0.1 for command and ::/0.0.0.0 for communication like originally proposed. If something of this fails, we can print a warning but continue.


Apart from this it sounds pretty crazy that Docker and Travis CI don't support IPv6 by default, since this is pretty modern technology 🤔

EDIT: Wording

@hsanjuan hsanjuan mentioned this pull request Feb 28, 2020
@hsanjuan
Copy link
Collaborator Author

I'll merge this as is. Enabling ipv6 with this as an addition to ipv4 is very easy and I don't see we should default to it or complicates things more right now, and now it would be just matter of updating the defaults which should be super easy.

@hsanjuan hsanjuan merged commit 531379b into master Feb 28, 2020
@hsanjuan hsanjuan deleted the RubenKelevra/issue_1000 branch February 28, 2020 16:16
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.

2 participants