forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Backport partial addrv2 support #3939
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bc74a40 net: improve encapsulation of CNetAddr (Vasil Dimov) Pull request description: Do not access `CNetAddr::ip` directly from `CService` methods. This improvement will help later when we change the type of `CNetAddr::ip` (in the BIP155 implementation). (chopped off from bitcoin#19031 to ease review) ACKs for top commit: dongcarl: ACK bc74a40 naumenkogs: ACK bc74a40 fjahr: Code review ACK bc74a40 laanwj: code review ACK bc74a40 jonatack: ACK bc74a40 jnewbery: ACK bc74a40 Tree-SHA512: 29a203905538e8311e3249b78565abe69ce36dc4ec239bec85c726c30e1a7b55b0aaf5c6659b676935008e068cfa53d716f7a598469064108daf130f94329a5d
Signed-off-by: pasta <[email protected]>
8be3f30 netaddress: Update CNetAddr for ORCHIDv2 (Carl Dong) Pull request description: ``` The original ORCHID prefix was deprecated as of 2014-03, the new ORCHIDv2 prefix was allocated by RFC7343 as of 2014-07. We did not consider the original ORCHID prefix routable, and I don't see any reason to consider the new one to be either. ``` Would like to know if people think this kind of thing is even worth keeping the codebase updated for. Perhaps it'd be nice to write a devtool to pull the csv from [here](https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml) and generate the code. ACKs for commit 8be3f3: laanwj: utACK 8be3f30 ryanofsky: utACK 8be3f30. Only change since last review is rebasing after bitcoin#15718 merge. Tree-SHA512: 7c93317f597b1a6c1443e12dd690010392edb9d72a479a8201970db7d3444fbb99a80b98026caad6fbfbebb455ab4035d2dde79bc9263bfd1d0398cd218392e1
ccef5d7 test: add two edge case tests for CSubNet (Vasil Dimov) Pull request description: This is chopped off from bitcoin#19031. It is needed because later 19031 modifies the related code and the tests ensure that no surprising changes in behavior sneak in. ACKs for top commit: practicalswift: ACK ccef5d7 -- more test coverage is better than less test coverage :) laanwj: ACK ccef5d7 hebasto: ACK ccef5d7, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 6d386672b6598aeddd33dabe3512e816cf548d5c1af56c4c9e6f897d513b62ba4659cde73405811a0df286ffee3a3f084ab7caf8e3a2086fa9ddecd1bdcb3c67
bcfebb6 net: save the network type explicitly in CNetAddr (Vasil Dimov) 100c64a net: document `enum Network` (Vasil Dimov) Pull request description: (chopped off from bitcoin#19031 to ease review) Before this change, we would analyze the contents of `CNetAddr::ip[16]` in order to tell which type is an address. Change this by introducing a new member `CNetAddr::m_net` that explicitly tells the type of the address. This is necessary because in BIP155 we will not be able to tell the address type by just looking at its raw representation (e.g. both TORv3 and I2P are "seemingly random" 32 bytes). As a side effect of this change we no longer need to store IPv4 addresses encoded as IPv6 addresses - we can store them in proper 4 bytes (will be done in a separate commit). Also the code gets somewhat simplified - instead of `memcmp(ip, pchIPv4, sizeof(pchIPv4)) == 0` we can use `m_net == NET_IPV4`. ACKs for top commit: troygiorshev: reACK bcfebb6 via `git range-diff master 64897c5 bcfebb6` jonatack: re-ACK bcfebb6 per `git diff 662bb25 bcfebb6`, code review, debug build/tests clean, ran bitcoind. laanwj: Code review ACK bcfebb6 Tree-SHA512: 9347e2a50feac617a994bfb46a8f77e31c236bde882e4fd4f03eea4766cd5110216f5f3d24dee91d25218bab7f8bb6e1d2d6212a44db9e34594299fd6ff7606b Signed-off-by: pasta <[email protected]> # Conflicts: # src/netaddress.cpp # src/netaddress.h
0e08366 to
888f36a
Compare
xdustinface
approved these changes
Jan 27, 2021
xdustinface
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
UdjinM6
approved these changes
Jan 28, 2021
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
backporting PRs pulled from bitcoin#19031
This is being backported ahead of time in order to not lose all tor support once torv2 is fully deprecated. see https://blog.torproject.org/v2-deprecation-timeline
I believe my serialization fixes in 90bcff9 are wrong.
Additionally, 0e08366 breaks the unit tests in a way I really don't understand