Skip to content

Commit ca05b28

Browse files
committed
Merge bitcoin#31859: test: Rename send_message to send_without_ping
fa9cf38 scripted-diff: test: Rename send_message to send_without_ping (MarcoFalke) fa43567 test: Prefer send_and_ping over send_message+sync_with_ping (MarcoFalke) Pull request description: `send_message` is problematic, because it is easy to forget a `sync_with_ping` (or other `wait_until`), leading to intermittent test failures. (Example: bitcoin#31837 (comment)) There are more uses of `send_and_ping` in the codebase than `send_message`, so in most cases `send_and_ping` is needed anyway. For the remaining cases, clearly document that no sync happens by renaming `send_message` to `send_without_ping`. ACKs for top commit: instagibbs: ACK bitcoin@fa9cf38 Tree-SHA512: 31caa6568d292ae3d3dda931a94aaa30cc1205ec2ef537a484393eb55687f86c212f1e751ac4a7636610bdf591502a50995dc63bf02f97be9fdc482072160b07
2 parents ab2df17 + fa9cf38 commit ca05b28

35 files changed

+159
-161
lines changed

test/functional/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ way is the use the `profile_with_perf` context manager, e.g.
183183
with node.profile_with_perf("send-big-msgs"):
184184
# Perform activity on the node you're interested in profiling, e.g.:
185185
for _ in range(10000):
186-
node.p2ps[0].send_message(some_large_message)
186+
node.p2ps[0].send_without_ping(some_large_message)
187187
```
188188

189189
To see useful textual output, run

test/functional/example_test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ def run_test(self):
184184
block.solve()
185185
block_message = msg_block(block)
186186
# Send message is used to send a P2P message to the node over our P2PInterface
187-
peer_messaging.send_message(block_message)
187+
peer_messaging.send_without_ping(block_message)
188188
self.tip = block.sha256
189189
blocks.append(self.tip)
190190
self.block_time += 1
@@ -209,7 +209,7 @@ def run_test(self):
209209
getdata_request = msg_getdata()
210210
for block in blocks:
211211
getdata_request.inv.append(CInv(MSG_BLOCK, block))
212-
peer_receiving.send_message(getdata_request)
212+
peer_receiving.send_without_ping(getdata_request)
213213

214214
# wait_until() will loop until a predicate condition is met. Use it to test properties of the
215215
# P2PInterface objects.

test/functional/feature_assumeutxo.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ def test_sync_from_assumeutxo_node(self, snapshot):
309309
msg = msg_headers()
310310
for block_num in range(1, miner.getblockcount()+1):
311311
msg.headers.append(from_hex(CBlockHeader(), miner.getblockheader(miner.getblockhash(block_num), verbose=False)))
312-
headers_provider_conn.send_message(msg)
312+
headers_provider_conn.send_without_ping(msg)
313313

314314
# Ensure headers arrived
315315
default_value = {'status': ''} # No status

test/functional/feature_assumevalid.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class BaseNode(P2PInterface):
5858
def send_header_for_blocks(self, new_blocks):
5959
headers_message = msg_headers()
6060
headers_message.headers = [CBlockHeader(b) for b in new_blocks]
61-
self.send_message(headers_message)
61+
self.send_without_ping(headers_message)
6262

6363

6464
class AssumeValidTest(BitcoinTestFramework):
@@ -80,7 +80,7 @@ def send_blocks_until_disconnected(self, p2p_conn):
8080
if not p2p_conn.is_connected:
8181
break
8282
try:
83-
p2p_conn.send_message(msg_block(self.blocks[i]))
83+
p2p_conn.send_without_ping(msg_block(self.blocks[i]))
8484
except IOError:
8585
assert not p2p_conn.is_connected
8686
break
@@ -157,7 +157,7 @@ def run_test(self):
157157

158158
# Send all blocks to node1. All blocks will be accepted.
159159
for i in range(2202):
160-
p2p1.send_message(msg_block(self.blocks[i]))
160+
p2p1.send_without_ping(msg_block(self.blocks[i]))
161161
# Syncing 2200 blocks can take a while on slow systems. Give it plenty of time to sync.
162162
p2p1.sync_with_ping(timeout=960)
163163
assert_equal(self.nodes[1].getblock(self.nodes[1].getbestblockhash())['height'], 2202)

test/functional/feature_maxuploadtarget.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ def run_test(self):
124124
# the test has been running so far).
125125
with self.nodes[0].assert_debug_log(expected_msgs=["historical block serving limit reached, disconnecting peer=0"]):
126126
for _ in range(3):
127-
p2p_conns[0].send_message(getdata_request)
127+
p2p_conns[0].send_without_ping(getdata_request)
128128
p2p_conns[0].wait_for_disconnect()
129129
assert_equal(len(self.nodes[0].getpeerinfo()), 2)
130130
self.log.info("Peer 0 disconnected after downloading old block too many times")
@@ -148,7 +148,7 @@ def run_test(self):
148148
# But if p2p_conns[1] tries for an old block, it gets disconnected too.
149149
getdata_request.inv = [CInv(MSG_BLOCK, big_old_block)]
150150
with self.nodes[0].assert_debug_log(expected_msgs=["historical block serving limit reached, disconnecting peer=1"]):
151-
p2p_conns[1].send_message(getdata_request)
151+
p2p_conns[1].send_without_ping(getdata_request)
152152
p2p_conns[1].wait_for_disconnect()
153153
assert_equal(len(self.nodes[0].getpeerinfo()), 1)
154154

@@ -198,7 +198,7 @@ def run_test(self):
198198

199199
self.log.info("Peer gets disconnected for a mempool request after limit is reached")
200200
with self.nodes[0].assert_debug_log(expected_msgs=["mempool request with bandwidth limit reached, disconnecting peer=0"]):
201-
peer.send_message(msg_mempool())
201+
peer.send_without_ping(msg_mempool())
202202
peer.wait_for_disconnect()
203203

204204
self.log.info("Test passing an unparsable value to -maxuploadtarget throws an error")

test/functional/feature_versionbits_warning.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def send_blocks_with_version(self, peer, numblocks, version):
4747
for _ in range(numblocks):
4848
block = create_block(tip, create_coinbase(height + 1), block_time, version=version)
4949
block.solve()
50-
peer.send_message(msg_block(block))
50+
peer.send_without_ping(msg_block(block))
5151
block_time += 1
5252
height += 1
5353
tip = block.sha256

test/functional/interface_usdt_net.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ def handle_misbehaving_connection(_, data, __):
473473
for _ in range(EXPECTED_MISBEHAVING_CONNECTIONS):
474474
testnode = P2PInterface()
475475
self.nodes[0].add_p2p_connection(testnode)
476-
testnode.send_message(msg)
476+
testnode.send_without_ping(msg)
477477
bpf.perf_buffer_poll(timeout=500)
478478
testnode.peer_disconnect()
479479

test/functional/p2p_addr_relay.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ def addr_received(self):
7676

7777
def on_version(self, message):
7878
self.send_version()
79-
self.send_message(msg_verack())
79+
self.send_without_ping(msg_verack())
8080
if (self.send_getaddr):
81-
self.send_message(msg_getaddr())
81+
self.send_without_ping(msg_getaddr())
8282

8383
def getaddr_received(self):
8484
return self.message_count['getaddr'] > 0
@@ -142,7 +142,7 @@ def oversized_addr_test(self):
142142

143143
msg = self.setup_addr_msg(1010)
144144
with self.nodes[0].assert_debug_log(['addr message size = 1010']):
145-
addr_source.send_message(msg)
145+
addr_source.send_without_ping(msg)
146146
addr_source.wait_for_disconnect()
147147

148148
self.nodes[0].disconnect_p2ps()

test/functional/p2p_addrfetch.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def run_test(self):
6262

6363
self.log.info("Check that answering with larger addr messages leads to disconnect")
6464
msg.addrs = [ADDR] * 2
65-
peer.send_message(msg)
65+
peer.send_without_ping(msg)
6666
peer.wait_for_disconnect(timeout=5)
6767

6868
self.log.info("Check timeout for addr-fetch peer that does not send addrs")

test/functional/p2p_addrv2_relay.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def run_test(self):
7979
self.log.info('Check disconnection when sending sendaddrv2 after verack')
8080
conn = self.nodes[0].add_p2p_connection(P2PInterface())
8181
with self.nodes[0].assert_debug_log(['sendaddrv2 received after verack, disconnecting peer=0']):
82-
conn.send_message(msg_sendaddrv2())
82+
conn.send_without_ping(msg_sendaddrv2())
8383
conn.wait_for_disconnect()
8484

8585
self.log.info('Create connection that sends addrv2 messages')
@@ -104,7 +104,7 @@ def run_test(self):
104104
self.log.info('Send too-large addrv2 message')
105105
msg.addrs = ADDRS * 101
106106
with self.nodes[0].assert_debug_log(['addrv2 message size = 1010']):
107-
addr_source.send_message(msg)
107+
addr_source.send_without_ping(msg)
108108
addr_source.wait_for_disconnect()
109109

110110

0 commit comments

Comments
 (0)