Skip to content

Conversation

@PastaPastaPasta
Copy link
Member

@PastaPastaPasta PastaPastaPasta commented Jan 16, 2021

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

laanwj and others added 6 commits January 16, 2021 01:21
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
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
Copy link

@xdustinface xdustinface left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 added this to the 17 milestone Jan 28, 2021
Copy link

@UdjinM6 UdjinM6 left a 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants