Skip to content
This repository has been archived by the owner on Aug 23, 2020. It is now read-only.

Fix: Always set the domain field of a neighbor #1604

Merged
merged 1 commit into from Sep 11, 2019

Conversation

luca-moser
Copy link
Member

Description

This PR alters the logic of neighbor properties and the getNeighbors() API call to fulfill following rules:

  • if the neighbor is connected and there's no mapping to a domain, the IP address will be set in the domain field
  • if the neighbor is connected and there's a mapping to a domain, the domain will be set in the domain field
  • if the neighbor is not connected, the neighbor was added using an URI with a domain (tcp://example.com:15600) and during the call to getNeighbors() the domain can't be resolved to an IP address, the domain field will contain the domain name and the address field will be empty
  • if the neighbor is not connected, the neighbor was added using an URI with an IP address (tcp://1.1.1.1:15600), the domain field will contain the IP address and the address field will contain the IP address + port combination
  • if the neighbor is not connected, the neighbor was added using an URI with a domain (tcp://example.com:15600) and during the call to getNeighbors() the domain could be resolved to an IP address, the domain field will contain the domain name and the address field will contain the IP address + port combination.

The domain field of a neighbor always either contains the domain name of the initial URI or the IP address. It is never empty.

Additionally, this PR stops the node from doing reverse lookups of IP addresses to gather a domain name, as often a not desired domain will be set.

Fixes # (issue)
#1536

Type of change

  • Bug fix (a non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@codepleb
Copy link

codepleb commented Sep 7, 2019

Thank you very much luca!

@GalRogozinski
Copy link
Contributor

Looks good!

If I am not mistaken, most of the regression tests for this bug can be created easily without adding any python. I think currently we can only add nodes via their IP though...

For the other cases where the neighbor is not added with the domain it should be a breeze

@jakubcech
Copy link
Contributor

👍 on removing the reverse lookup

@GalRogozinski
Copy link
Contributor

Even though simple regression tests should be added to bugs we solve (especially when they are trivial) I am opening #1611 so the team can focus on tip selection.

In the meanwhile I'll be merging this before we will have conflicts to solve.

@GalRogozinski GalRogozinski merged commit 490bf73 into iotaledger:dev Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants