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

QSP-16 Fix Support for IPv6 #6363

Merged
merged 28 commits into from
Sep 16, 2020
Merged

QSP-16 Fix Support for IPv6 #6363

merged 28 commits into from
Sep 16, 2020

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented Jun 23, 2020

What type of PR is this?

Bug Fix Raised from Audit

What does this PR do? Why is it needed?

  • Fixes our IPv6 support by accounting for ipv6 addresses when converting
    an external ENR in convertToSingleMultiAddr().
  • Add Regression test for this
  • Add a common util to retrieve both ipv4/ipv6 addresses
  • Verify it works during runtime.

Which issues(s) does this PR fix?

Part of #6327

Other notes for review

@nisdas nisdas added the Audit label Jun 23, 2020
@stale
Copy link

stale bot commented Jul 1, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale There hasn't been any activity here in some time... label Jul 1, 2020
@nisdas nisdas removed the Stale There hasn't been any activity here in some time... label Jul 3, 2020
@stale
Copy link

stale bot commented Jul 11, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale There hasn't been any activity here in some time... label Jul 11, 2020
@stale
Copy link

stale bot commented Jul 25, 2020

This pull request has been closed due to inactivity. Please reopen this pull request if you would like to continue working on it.

@stale stale bot closed this Jul 25, 2020
@prestonvanloon prestonvanloon removed the Stale There hasn't been any activity here in some time... label Sep 10, 2020
@prestonvanloon prestonvanloon self-assigned this Sep 10, 2020
@codecov
Copy link

codecov bot commented Sep 10, 2020

Codecov Report

Merging #6363 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6363   +/-   ##
=======================================
  Coverage   61.90%   61.90%           
=======================================
  Files         411      411           
  Lines       33060    33060           
=======================================
  Hits        20466    20466           
  Misses       9695     9695           
  Partials     2899     2899           

@nisdas nisdas marked this pull request as ready for review September 14, 2020 14:28
@nisdas nisdas requested a review from a team as a code owner September 14, 2020 14:28

// ExternalIPv6 retrieves any allocated IPv6 addresses
// from the accessible network interfaces.
func ExternalIPv6() (string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

I realized ExternalIPv6() is not covered in external_ip_test.go . Should we cover it?

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue would be in bazels sandbox the ipv6 address isnt availble from the accessible network interface which makes it hard to test something like this.

@nisdas nisdas merged commit 56d6e05 into master Sep 16, 2020
@delete-merged-branch delete-merged-branch bot deleted the fixIPv6 branch September 16, 2020 00:17
@nisdas nisdas mentioned this pull request Sep 17, 2020
3 tasks
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.

3 participants