Repository: bitcoin/bitcoin
File: test/functional/test_framework/p2p.py (Python)

Code before:
        def peer_protocol():
            """Returns a function that does the protocol handling for a new
            connection. To allow different connections to have different
            behaviors, the protocol function is first put in the cls.protos
            dict. When the connection is made, the function removes the
            protocol function from that dict, and returns it so the event loop
            can start executing it."""
            response = cls.protos.get((addr, port))
            # remove protocol function from dict only when reconnection doesn't need to happen/already happened
            if not proto.reconnect:
                cls.protos[(addr, port)] = None
            return response

        if (addr, port) not in cls.listeners:
            # When creating a listener on a given (addr, port) we only need to
            # do it once. If we want different behaviors for different
            # connections, we can accomplish this by providing different
            # `proto` functions

            listener = await cls.network_event_loop.create_server(peer_protocol, addr, port)
            # When port=0, the OS assigns an available ephemeral port. Retrieve
            # the actual port so callers know where the server is listening.
            actual_port = listener.sockets[0].getsockname()[1]
            logger.debug("Listening server on %s:%d should be started" % (addr, actual_port))
            cls.listeners[(addr, actual_port)] = listener
            port = actual_port

        cls.protos[(addr, port)] = proto
        callback(addr, port)


class P2PDataStore(P2PInterface):
    """A P2P data store class.

    Keeps a block and transaction store and responds correctly to getdata and getheaders requests."""

    def __init__(self):
        super().__init__()
        # store of blocks. key is block hash, value is a CBlock object
        self.block_store = {}
        self.last_block_hash = ''
        # store of txs. key is txid, value is a CTransaction object
        self.tx_store = {}
        self.getdata_requests = []

    def on_getdata(self, message):
        """Check for the tx/block in our stores and if found, reply with MSG_TX or MSG_BLOCK."""
        for inv in message.inv:
            self.getdata_requests.append(inv.hash)
            invtype = inv.type & MSG_TYPE_MASK
            if (invtype == MSG_TX or invtype == MSG_WTX) and inv.hash in self.tx_store.keys():

Review comment (bug):
```py
776     async def create_listen_server(cls, addr, port, callback, proto):
777         def peer_protocol():
778             """Returns a function that does the protocol handling for a new
779             connection. To allow different connections to have different
780             behaviors, the protocol function is first put in the cls.protos
781             dict. When the connection is made, the function removes the
782             protocol function from that dict, and returns it so the event loop
783             can start executing it."""
784             response = cls.protos.get((addr, port))
785             # remove protocol function from dict only when reconnection doesn't need to happen/already happened
786             if not proto.reconnect:
787                 cls.protos[(addr, port)] = None
788             return response
789 
790         if (addr, port) not in cls.listeners:
791             # When creating a listener on a given (addr, port) we only need to
792             # do it once. If we want different behaviors for different
793             # connections, we can accomplish this by providing different
794             # `proto` functions                                                    
795 
796             listener = await cls.network_event_loop.create_server(peer_protocol, addr, port)
797             # When port=0, the OS assigns an available ephemeral port. Retrieve
798             # the actual port so callers know where the server is listening.
799             actual_port = listener.sockets[0].getsockname()[1]
800             logger.debug("Listening server on %s:%d should be started" % (addr, actual_port))
801             cls.listeners[(addr, actual_port)] = listener
802             port = actual_port
803 
804         cls.protos[(addr, port)] = proto
805         callback(addr, port)
```

Is it not the case that inside `peer_protocol()` and on line `790` `port` will be `0` which is unintended? That is, those locations expect the actual port?

Code after:
        def peer_protocol():
            """Returns a function that does the protocol handling for a new
            connection. To allow different connections to have different
            behaviors, the protocol function is first put in the cls.protos
            dict. When the connection is made, the function removes the
            protocol function from that dict, and returns it so the event loop
            can start executing it."""
            response = cls.protos.get((addr, port))
            # remove protocol function from dict only when reconnection doesn't need to happen/already happened
            if not proto.reconnect:
                cls.protos[(addr, port)] = None
            return response

        if port == 0 or (addr, port) not in cls.listeners:
            # When creating a listener on a given (addr, port) we only need to
            # do it once. If we want different behaviors for different
            # connections, we can accomplish this by providing different
            # `proto` functions

            listener = await cls.network_event_loop.create_server(peer_protocol, addr, port)
            port = listener.sockets[0].getsockname()[1]
            logger.debug("Listening server on %s:%d should be started" % (addr, port))
            cls.listeners[(addr, port)] = listener

        cls.protos[(addr, port)] = proto
        callback(addr, port)


class P2PDataStore(P2PInterface):
    """A P2P data store class.

    Keeps a block and transaction store and responds correctly to getdata and getheaders requests."""

    def __init__(self):
        super().__init__()
        # store of blocks. key is block hash, value is a CBlock object
        self.block_store = {}
        self.last_block_hash = ''
        # store of txs. key is txid, value is a CTransaction object
        self.tx_store = {}
        self.getdata_requests = []

    def on_getdata(self, message):
        """Check for the tx/block in our stores and if found, reply with MSG_TX or MSG_BLOCK."""
        for inv in message.inv:
            self.getdata_requests.append(inv.hash)
            invtype = inv.type & MSG_TYPE_MASK
            if (invtype == MSG_TX or invtype == MSG_WTX) and inv.hash in self.tx_store.keys():
                self.send_without_ping(msg_tx(self.tx_store[inv.hash]))
            elif invtype == MSG_BLOCK and inv.hash in self.block_store.keys():
                self.send_without_ping(msg_block(self.block_store[inv.hash]))