mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-03-16 16:32:47 +00:00
Merge bitcoin/bitcoin#34750: test: fix addr relay test silently passing and other improvements
57bfa864fe69ea5610399f9db60cf2299930703a test: use static methods and clarify comment in addr_relay (stratospher)
7ee8c0abc629f0658b6c36f36b5da11c51cbb39d test: protect outbound connection from eviction in getaddr_test (stratospher)
ecb5ce6e76ef9391d636e7e4f6be7e3f7ed05550 test: fix addr relay test silent pass and wrong peerinfo index (stratospher)
Pull request description:
couple of improvements in the addr relay test:
- fixes the silent test pass discovered in https://github.com/bitcoin/bitcoin/pull/34717#issuecomment-3990585763 (will remove this if that PR gets merged - the test doesn't fail even though #34717 changes the behaviour)
- correct wrong peerinfo index
- prevent intermittent disconnection warnings like the one shown below by protecting outbound peer from `ConsiderEviction`
```
TestFramework (INFO): Check that we answer getaddr messages only once per connection
TestFramework.p2p (WARNING): Connection lost to 127.0.0.1:58829 due to [Errno 54] Connection reset by peer
```
- remove a no longer applicable test comment since we don't need to send initial GETADDR for intial self announcement anymore
ACKs for top commit:
Bortlesboat:
ACK 57bfa864fe69. Ran both `p2p_addr_relay.py` and `p2p_addr_selfannouncement.py` locally, both pass. Good catch on the stale `peerinfo` reference in `inbound_blackhole_tests` — that would silently check the wrong peer.
naiyoma:
ACK 57bfa864fe69ea5610399f9db60cf2299930703a
mzumsande:
Code Review ACK 57bfa864fe69ea5610399f9db60cf2299930703a
Tree-SHA512: 22e4f87f66569bfa629a68a8b440cd21b5285c6dad6eb7926514f2d74e16fe3711525b264f82765c83020be976a0438b8d2ab1e48e7c0b7d85437ee672d52324
This commit is contained in:
commit
5608b8ce9e
@ -11,9 +11,12 @@ import time
|
||||
|
||||
from test_framework.messages import (
|
||||
CAddress,
|
||||
CBlockHeader,
|
||||
msg_addr,
|
||||
msg_getaddr,
|
||||
msg_headers,
|
||||
msg_verack,
|
||||
from_hex,
|
||||
)
|
||||
from test_framework.p2p import (
|
||||
P2PInterface,
|
||||
@ -178,6 +181,9 @@ class AddrTest(BitcoinTestFramework):
|
||||
|
||||
self.log.info('Check relay of addresses received from outbound peers')
|
||||
inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver(test_addr_contents=True, send_getaddr=False))
|
||||
# Send an empty ADDR message to initialize address relay on this connection.
|
||||
inbound_peer.send_and_ping(msg_addr())
|
||||
|
||||
full_outbound_peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=0, connection_type="outbound-full-relay")
|
||||
msg = self.setup_addr_msg(2)
|
||||
self.send_addr_msg(full_outbound_peer, msg, [inbound_peer])
|
||||
@ -188,9 +194,6 @@ class AddrTest(BitcoinTestFramework):
|
||||
# of the outbound peer which is often sent before the GETADDR response.
|
||||
assert_equal(inbound_peer.num_ipv4_received, 0)
|
||||
|
||||
# Send an empty ADDR message to initialize address relay on this connection.
|
||||
inbound_peer.send_and_ping(msg_addr())
|
||||
|
||||
self.log.info('Check that subsequent addr messages sent from an outbound peer are relayed')
|
||||
msg2 = self.setup_addr_msg(2)
|
||||
self.send_addr_msg(full_outbound_peer, msg2, [inbound_peer])
|
||||
@ -248,8 +251,9 @@ class AddrTest(BitcoinTestFramework):
|
||||
blackhole_peer.send_and_ping(msg_addr())
|
||||
|
||||
# Confirm node has now received addr-related messages from blackhole peer
|
||||
assert_greater_than(self.sum_addr_messages(peerinfo[1]['bytesrecv_per_msg']), 0)
|
||||
assert_equal(self.nodes[0].getpeerinfo()[2]['addr_relay_enabled'], True)
|
||||
peerinfo = self.nodes[0].getpeerinfo()
|
||||
assert_greater_than(self.sum_addr_messages(peerinfo[2]['bytesrecv_per_msg']), 0)
|
||||
assert_equal(peerinfo[2]['addr_relay_enabled'], True)
|
||||
|
||||
msg = self.setup_addr_msg(2)
|
||||
self.send_addr_msg(addr_source, msg, [receiver_peer, blackhole_peer])
|
||||
@ -272,14 +276,20 @@ class AddrTest(BitcoinTestFramework):
|
||||
full_outbound_peer.sync_with_ping()
|
||||
assert full_outbound_peer.getaddr_received()
|
||||
|
||||
# to avoid the node evicting the outbound peer, protect it by announcing the most recent header to it
|
||||
tip_header = from_hex(CBlockHeader(), self.nodes[0].getblockheader(self.nodes[0].getbestblockhash(), False))
|
||||
full_outbound_peer.send_and_ping(msg_headers([tip_header]))
|
||||
|
||||
self.log.info('Check that we do not send a getaddr message to a block-relay-only or inbound peer')
|
||||
block_relay_peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=1, connection_type="block-relay-only")
|
||||
block_relay_peer.sync_with_ping()
|
||||
assert_equal(block_relay_peer.getaddr_received(), False)
|
||||
block_relay_peer.send_and_ping(msg_headers([tip_header]))
|
||||
|
||||
inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver(send_getaddr=False))
|
||||
inbound_peer.sync_with_ping()
|
||||
assert_equal(inbound_peer.getaddr_received(), False)
|
||||
assert_equal(inbound_peer.addr_received(), False)
|
||||
|
||||
self.log.info('Check that we answer getaddr messages only from inbound peers')
|
||||
# Add some addresses to addrman
|
||||
@ -392,8 +402,7 @@ class AddrTest(BitcoinTestFramework):
|
||||
def get_nodes_that_received_addr(self, peer, receiver_peer, addr_receivers,
|
||||
time_interval_1, time_interval_2):
|
||||
|
||||
# Clean addr response related to the initial getaddr. There is no way to avoid initial
|
||||
# getaddr because the peer won't self-announce then.
|
||||
# Clean addr response related to the initial getaddr.
|
||||
for addr_receiver in addr_receivers:
|
||||
addr_receiver.num_ipv4_received = 0
|
||||
|
||||
|
||||
@ -78,14 +78,16 @@ class AddrSelfAnnouncementTest(BitcoinTestFramework):
|
||||
self.self_announcement_test(outbound=True, addrv2=False)
|
||||
self.self_announcement_test(outbound=True, addrv2=True)
|
||||
|
||||
def inbound_connection_open_assertions(self, addr_receiver):
|
||||
@staticmethod
|
||||
def inbound_connection_open_assertions(addr_receiver):
|
||||
# In response to a GETADDR, we expect a message with the self-announcement
|
||||
# and an addr message containing the GETADDR response.
|
||||
assert_equal(addr_receiver.self_announcements_received, 1)
|
||||
assert_equal(addr_receiver.addr_messages_received, 2)
|
||||
assert_greater_than(addr_receiver.addresses_received, 1)
|
||||
|
||||
def outbound_connection_open_assertions(self, addr_receiver):
|
||||
@staticmethod
|
||||
def outbound_connection_open_assertions(addr_receiver):
|
||||
# We expect only the self-announcement.
|
||||
assert_equal(addr_receiver.self_announcements_received, 1)
|
||||
assert_equal(addr_receiver.addr_messages_received, 1)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user