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

addrmgr: Track network types and remove TorV2 support #3406

Merged
merged 3 commits into from
Jul 20, 2024

Conversation

matthawkins90
Copy link
Contributor

This heavily references the work started in #2627. However, the work will be broken out into smaller chunks. Ultimately, the goal in the near future is to refactor Addrmgr to remove TorV2 support, and add support for TorV3. However, this PR doesn't include any TorV3 work yet.

The first commit enables tracking of address types, and includes several function refactors to improve readability. The new types are defined in network.go, and then constructed in netaddress.go. The rest of the changes propagate outward from there.

The second commit removes TorV2 support.

The third commit adds new functions: ParseHost and NewNetAddressFromParams. These functions will eventually enable two desirable features:

  • The ability to parse and construct any type of network address, including TorV3, I2P, or other
  • The ability to remove DNS lookups from Addrmgr (and move it into Server.go).

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Thanks for the submission. I've reviewed the first commit and it looks pretty good overall. I pointed out a few minor things inline.

I'll finish the remaining commits tomorrow.

addrmgr/knownaddress.go Outdated Show resolved Hide resolved
addrmgr/knownaddress.go Outdated Show resolved Hide resolved
addrmgr/knownaddress.go Outdated Show resolved Hide resolved
addrmgr/netaddress.go Outdated Show resolved Hide resolved
addrmgr/netaddress_test.go Outdated Show resolved Hide resolved
addrmgr/doc.go Outdated Show resolved Hide resolved
addrmgr/addrmanager_test.go Outdated Show resolved Hide resolved
addrmgr/addrmanager_test.go Outdated Show resolved Hide resolved
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Alright, I've finished reviewing. I discussed it more inline, but this seems to be incorrectly presuming hosts are IPs and that is not necessarily the case at all.

addrmgr/addrmanager.go Outdated Show resolved Hide resolved
addrmgr/addrmanager.go Show resolved Hide resolved
addrmgr/addrmanager.go Outdated Show resolved Hide resolved
addrmgr/netaddress.go Show resolved Hide resolved
This commit adds support for determining and tracking the type of a
network address when constructed. While here, several functions are
refactored for easier reading and interpretability.

The types are first defined in network.go, and then constructed in
netaddress.go. The rest of the changes propagate outward from there.
Minimal changes were made to anything related to TorV2, because it will
be removed in a future commit.
@matthawkins90 matthawkins90 force-pushed the addrmgr_types branch 2 times, most recently from 6ccac1f to 9e40429 Compare July 19, 2024 03:21
This commit creates two new functions: EncodeHost and
NewNetAddressFromParams. The goal is to allow easier creation of net
addresses without needing to resort to DNS lookups. DNS lookups will be
removed from addrmgr.
@davecgh davecgh merged commit 0d902af into decred:master Jul 20, 2024
2 checks passed
@matthawkins90 matthawkins90 deleted the addrmgr_types branch July 20, 2024 22:32
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