test: add support for all networks in `deserialize_v2` #27295

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2023-03-improv-deserialize-v2 changing 1 files +33 −5
  1. brunoerg commented at 11:29 PM on March 21, 2023: contributor

    Fixes #27140

  2. DrahtBot commented at 11:30 PM on March 21, 2023: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK jonatack

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #27452 (test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py by pinheadmz)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. DrahtBot added the label Tests on Mar 21, 2023
  4. fanquake commented at 11:20 AM on March 22, 2023: member

    https://cirrus-ci.com/task/6600092300345344

     test  2023-03-21T23:55:59.273000Z TestFramework (ERROR): Unexpected exception caught during testing 
                                       Traceback (most recent call last):
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\test_framework.py", line 132, in main
                                           self.run_test()
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\p2p_segwit.py", line 285, in run_test
                                           self.test_standardness_v0()
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\p2p_segwit.py", line 112, in func_wrapper
                                           func(self, *args, **kwargs)
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\p2p_segwit.py", line 660, in test_standardness_v0
                                           self.generate(self.nodes[0], 1)
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\test_framework.py", line 665, in generate
                                           sync_fun() if sync_fun else self.sync_all()
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\test_framework.py", line 729, in sync_all
                                           self.sync_blocks(nodes)
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\test_framework.py", line 694, in sync_blocks
                                           best_hash = [x.getbestblockhash() for x in rpc_connections]
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\test_framework.py", line 694, in <listcomp>
                                           best_hash = [x.getbestblockhash() for x in rpc_connections]
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\coverage.py", line 49, in __call__
                                           return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\authproxy.py", line 147, in __call__
                                           response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
                                         File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\authproxy.py", line 110, in _request
                                           self.__conn.request(method, path, postdata, headers)
                                         File "C:\Python39\lib\http\client.py", line 1257, in request
                                           self._send_request(method, url, body, headers, encode_chunked)
                                         File "C:\Python39\lib\http\client.py", line 1303, in _send_request
                                           self.endheaders(body, encode_chunked=encode_chunked)
                                         File "C:\Python39\lib\http\client.py", line 1252, in endheaders
                                           self._send_output(message_body, encode_chunked=encode_chunked)
                                         File "C:\Python39\lib\http\client.py", line 1012, in _send_output
                                           self.send(msg)
                                         File "C:\Python39\lib\http\client.py", line 952, in send
                                           self.connect()
                                         File "C:\Python39\lib\http\client.py", line 925, in connect
                                           self.sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
                                       OSError: [WinError 10022] An invalid argument was supplied
     test  2023-03-21T23:55:59.280000Z TestFramework (DEBUG): Closing down network thread 
    
  5. maflcko commented at 11:23 AM on March 22, 2023: member
  6. brunoerg force-pushed on Mar 22, 2023
  7. brunoerg marked this as ready for review on Mar 22, 2023
  8. brunoerg commented at 6:49 PM on March 22, 2023: contributor

    @fanquake That error seems to be unrelated to this PR

  9. in test/functional/test_framework/messages.py:329 in a4111b7ea9 outdated
     323 |              self.ip = b32encode(addr_bytes)[0:-len(self.I2P_PAD)].decode("ascii").lower() + ".b32.i2p"
     324 | -
     325 | +        elif self.net == self.NET_CJDNS:
     326 | +           self.ip = ":".join(["{:02x}".format(b) for b in addr_bytes])
     327 | +        elif self.net in (self.NET_TORV2, self.NET_TORV3):
     328 | +            self.ip = b32encode(addr_bytes).decode('ascii') + '.onion'
    


    pinheadmz commented at 2:50 PM on April 12, 2023:

    Is this correct for torv3? I had to add version and checksum in https://github.com/bitcoin/bitcoin/pull/27452


    brunoerg commented at 6:40 PM on April 13, 2023:

    You're correct, seems better your approach.

  10. test: add support for all networks in `deserialize_v2`
    Co-authored-by: Matthew Zipkin <pinheadmz@gmail.com>
    8ef33e2dfe
  11. brunoerg force-pushed on Apr 13, 2023
  12. brunoerg commented at 6:40 PM on April 13, 2023: contributor

    Force-pushed changing the approach for torv3 addresses, added @pinheadmz as co-author.

  13. in test/functional/test_framework/messages.py:314 in 8ef33e2dfe
     308 | @@ -293,17 +309,29 @@ def deserialize_v2(self, f):
     309 |          self.nServices = deser_compact_size(f)
     310 |  
     311 |          self.net = struct.unpack("B", f.read(1))[0]
     312 | -        assert self.net in (self.NET_IPV4, self.NET_I2P)
     313 | +        assert self.net in (self.NET_IPV4, self.NET_IPV6,
     314 | +                            self.NET_I2P, self.NET_CJDNS,
     315 | +                            self.NET_TORV2, self.NET_TORV3)
    


    jonatack commented at 10:15 PM on June 22, 2023:

    This line seems better as done in #27452 with assert self.net in self.ADDRV2_NET_NAME

  14. in test/functional/test_framework/messages.py:245 in 8ef33e2dfe
     240 | @@ -237,16 +241,28 @@ class CAddress:
     241 |  
     242 |      # see https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki
     243 |      NET_IPV4 = 1
     244 | +    NET_IPV6 = 2
     245 | +    NET_TORV2 = 3
    


    jonatack commented at 10:18 PM on June 22, 2023:

    As also mentioned in #27452 (comment), we can drop the torv2 code entirely. Tor v2 been removed from the Tor network and isn't coming back.

  15. jonatack commented at 10:26 PM on June 22, 2023: contributor

    Concept ACK. This is very similar to a subset of the work in #27452, though a bit less complete here for the messages.py changes.

    I'm not sure how these changes were coordinated/worked on, but maybe this could be a separate commit in the other pull, which could possibly be split into preparatory and implementation commits.

  16. brunoerg commented at 4:53 PM on June 23, 2023: contributor

    Closing this in favor of #27452

  17. brunoerg closed this on Jun 23, 2023

  18. bitcoin locked this on Jun 22, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-19 15:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me