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

Revert "reduce redundant memory allocatio - resolves btcsuite/btcd#1699" #1804

Merged
merged 1 commit into from
Mar 30, 2022

Conversation

Crypt-iQ
Copy link
Collaborator

@Crypt-iQ Crypt-iQ commented Feb 4, 2022

This reverts commit 780cc08.

Oversight on my part, that extra byte is actually used. Without this, DNS resolution via tor proxy won't work

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1797030693

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 79.676%

Totals Coverage Status
Change from base Build 1780615008: 0.0%
Covered Lines: 1231
Relevant Lines: 1545

💛 - Coveralls

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 🌳

Confirmed that this breaks Tor resolution as is, and this PR resolves things. The issue is that 2 bytes are used for the port (with only one explicitly set), so it's 7 bytes total. We should also update the method to be able to parse a response that's an IPv6 address. I'll make an issue to track that.

@Roasbeef Roasbeef merged commit 0742662 into btcsuite:master Mar 30, 2022
@Crypt-iQ Crypt-iQ deleted the tor_resolver_fix branch March 30, 2022 20:42
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