Skip to content

Commit 75ed673

Browse files
committed
Merge bitcoin/bitcoin#33048: test: reduce runtime of p2p_opportunistic_1p1c.py
eb65f57 [test] setmocktime instead of waiting in 1p1c tests (glozow) 70772dd [test] cut the number of transactions involved in 1p1c DoS tests (glozow) Pull request description: It was brought to my attention that the runtime of this test is Too Damn High. The test is slow due to the many `wait_for_getdata`s with delays (inbound peer + txid request) and the large volume of messages sent in the dos-related tests. This PR cuts the runtime by about 60% by reducing the number of messages/transactions and using `setmocktime` instead of waiting. On my machine, master: ``` 84.51s user 1.55s system 57% cpu 2:28.53 total ``` After first commit (about 1min faster): ``` 28.29s user 0.88s system 35% cpu 1:22.84 total ``` After second commit (about 30sec faster): ``` 28.17s user 0.87s system 59% cpu 49.082 total ``` Reviewers should verify that the transactions in the DoS tests are still enough to cause evictions, and that the `bumpmocktime` amounts are not more than necessary. Alternatives: - If we don't like mocking the times, we can use outbound connections for all the peers. However, that approach won't improve the runtime as much because we impose a 2-second delay on all txid requests regardless of peer type. - Note that `noban_tx_relay` is not relevant for this test because all delays are related to downloading, not announcing. ACKs for top commit: achow101: ACK eb65f57 w0xlt: ACK bitcoin/bitcoin@eb65f57 Tree-SHA512: 6ffe1f9e5144653e2ded744cec9ddb62ad728c587705542565400a0e8f1fba4843aced4e0d929843874ca7f56f670f5871b7e009ff6be58b791ab24d2e6fcc0e
2 parents 24246c3 + eb65f57 commit 75ed673

File tree

1 file changed

+35
-24
lines changed

1 file changed

+35
-24
lines changed

test/functional/p2p_opportunistic_1p1c.py

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ def create_tx_below_mempoolminfee(self, wallet, utxo_to_spend=None):
9191
def test_basic_child_then_parent(self):
9292
node = self.nodes[0]
9393
self.log.info("Check that opportunistic 1p1c logic works when child is received before parent")
94+
node.setmocktime(int(time.time()))
9495

9596
low_fee_parent = self.create_tx_below_mempoolminfee(self.wallet)
9697
high_fee_child = self.wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=20*FEERATE_1SAT_VB)
@@ -100,11 +101,13 @@ def test_basic_child_then_parent(self):
100101
# 1. Child is received first (perhaps the low feerate parent didn't meet feefilter or the requests were sent to different nodes). It is missing an input.
101102
high_child_wtxid_int = high_fee_child["tx"].wtxid_int
102103
peer_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=high_child_wtxid_int)]))
104+
node.bumpmocktime(NONPREF_PEER_TX_DELAY)
103105
peer_sender.wait_for_getdata([high_child_wtxid_int])
104106
peer_sender.send_and_ping(msg_tx(high_fee_child["tx"]))
105107

106108
# 2. Node requests the missing parent by txid.
107109
parent_txid_int = int(low_fee_parent["txid"], 16)
110+
node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
108111
peer_sender.wait_for_getdata([parent_txid_int])
109112

110113
# 3. Sender relays the parent. Parent+Child are evaluated as a package and accepted.
@@ -120,6 +123,7 @@ def test_basic_child_then_parent(self):
120123
@cleanup
121124
def test_basic_parent_then_child(self, wallet):
122125
node = self.nodes[0]
126+
node.setmocktime(int(time.time()))
123127
low_fee_parent = self.create_tx_below_mempoolminfee(wallet)
124128
high_fee_child = wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=20*FEERATE_1SAT_VB)
125129

@@ -146,6 +150,7 @@ def test_basic_parent_then_child(self, wallet):
146150
# 3. Node requests the missing parent by txid.
147151
# It should do so even if it has previously rejected that parent for being too low feerate.
148152
parent_txid_int = int(low_fee_parent["txid"], 16)
153+
node.bumpmocktime(TXID_RELAY_DELAY)
149154
peer_sender.wait_for_getdata([parent_txid_int])
150155

151156
# 4. Sender re-relays the parent. Parent+Child are evaluated as a package and accepted.
@@ -159,6 +164,7 @@ def test_basic_parent_then_child(self, wallet):
159164
@cleanup
160165
def test_low_and_high_child(self, wallet):
161166
node = self.nodes[0]
167+
node.setmocktime(int(time.time()))
162168
low_fee_parent = self.create_tx_below_mempoolminfee(wallet)
163169
# This feerate is above mempoolminfee, but not enough to also bump the low feerate parent.
164170
feerate_just_above = node.getmempoolinfo()["mempoolminfee"]
@@ -189,6 +195,7 @@ def test_low_and_high_child(self, wallet):
189195

190196
# 3. Node requests the orphan's missing parent.
191197
parent_txid_int = int(low_fee_parent["txid"], 16)
198+
node.bumpmocktime(TXID_RELAY_DELAY)
192199
peer_sender.wait_for_getdata([parent_txid_int])
193200

194201
# 4. The low parent + low child are submitted as a package. They are not accepted due to low package feerate.
@@ -216,6 +223,7 @@ def test_low_and_high_child(self, wallet):
216223
# 6. Node requests the orphan's parent, even though it has already been rejected, both by
217224
# itself and with a child. This is necessary, otherwise high_fee_child can be censored.
218225
parent_txid_int = int(low_fee_parent["txid"], 16)
226+
node.bumpmocktime(TXID_RELAY_DELAY)
219227
peer_sender.wait_for_getdata([parent_txid_int])
220228

221229
# 7. The low feerate parent + high feerate child are submitted as a package.
@@ -231,6 +239,7 @@ def test_low_and_high_child(self, wallet):
231239
def test_orphan_consensus_failure(self):
232240
self.log.info("Check opportunistic 1p1c logic requires parent and child to be from the same peer")
233241
node = self.nodes[0]
242+
node.setmocktime(int(time.time()))
234243
low_fee_parent = self.create_tx_below_mempoolminfee(self.wallet)
235244
coin = low_fee_parent["new_utxo"]
236245
address = node.get_deterministic_priv_key().address
@@ -246,11 +255,13 @@ def test_orphan_consensus_failure(self):
246255
# 1. Child is received first. It is missing an input.
247256
child_wtxid_int = tx_orphan_bad_wit.wtxid_int
248257
bad_orphan_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=child_wtxid_int)]))
258+
node.bumpmocktime(NONPREF_PEER_TX_DELAY)
249259
bad_orphan_sender.wait_for_getdata([child_wtxid_int])
250260
bad_orphan_sender.send_and_ping(msg_tx(tx_orphan_bad_wit))
251261

252262
# 2. Node requests the missing parent by txid.
253263
parent_txid_int = int(low_fee_parent["txid"], 16)
264+
node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
254265
bad_orphan_sender.wait_for_getdata([parent_txid_int])
255266

256267
# 3. A different peer relays the parent. Package is not evaluated because the transactions
@@ -273,6 +284,8 @@ def test_orphan_consensus_failure(self):
273284
def test_parent_consensus_failure(self):
274285
self.log.info("Check opportunistic 1p1c logic with consensus-invalid parent causes disconnect of the correct peer")
275286
node = self.nodes[0]
287+
node.setmocktime(int(time.time()))
288+
276289
low_fee_parent = self.create_tx_below_mempoolminfee(self.wallet)
277290
high_fee_child = self.wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=999*FEERATE_1SAT_VB)
278291

@@ -287,11 +300,13 @@ def test_parent_consensus_failure(self):
287300
# 1. Child is received first. It is missing an input.
288301
child_wtxid_int = high_fee_child["tx"].wtxid_int
289302
package_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=child_wtxid_int)]))
303+
node.bumpmocktime(NONPREF_PEER_TX_DELAY)
290304
package_sender.wait_for_getdata([child_wtxid_int])
291305
package_sender.send_and_ping(msg_tx(high_fee_child["tx"]))
292306

293307
# 2. Node requests the missing parent by txid.
294308
parent_txid_int = tx_parent_bad_wit.txid_int
309+
node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
295310
package_sender.wait_for_getdata([parent_txid_int])
296311

297312
# 3. A different node relays the parent. The parent is first evaluated by itself and
@@ -309,6 +324,7 @@ def test_parent_consensus_failure(self):
309324
# It can send the "real" parent transaction, and the package is accepted.
310325
parent_wtxid_int = low_fee_parent["tx"].wtxid_int
311326
package_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=parent_wtxid_int)]))
327+
node.bumpmocktime(NONPREF_PEER_TX_DELAY)
312328
package_sender.wait_for_getdata([parent_wtxid_int])
313329
package_sender.send_and_ping(msg_tx(low_fee_parent["tx"]))
314330

@@ -355,6 +371,7 @@ def test_multiple_parents(self):
355371
def test_other_parent_in_mempool(self):
356372
self.log.info("Check opportunistic 1p1c works when part of a 2p1c (child already has another parent in mempool)")
357373
node = self.nodes[0]
374+
node.setmocktime(int(time.time()))
358375

359376
# Grandparent will enter mempool by itself
360377
grandparent_high = self.wallet.create_self_transfer(fee_rate=FEERATE_1SAT_VB*10, confirmed_only=True)
@@ -383,6 +400,7 @@ def test_other_parent_in_mempool(self):
383400

384401
# 4. Node requests parent_low.
385402
parent_low_txid_int = int(parent_low["txid"], 16)
403+
node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
386404
peer_sender.wait_for_getdata([parent_low_txid_int])
387405
peer_sender.send_and_ping(msg_tx(parent_low["tx"]))
388406

@@ -410,9 +428,10 @@ def test_orphanage_dos_large(self):
410428

411429
peer_normal = node.add_p2p_connection(P2PInterface())
412430
peer_doser = node.add_p2p_connection(P2PInterface())
431+
num_individual_dosers = 10
413432

414433
self.log.info("Create very large orphans to be sent by DoSy peers (may take a while)")
415-
large_orphans = [create_large_orphan() for _ in range(100)]
434+
large_orphans = [create_large_orphan() for _ in range(50)]
416435
# Check to make sure these are orphans, within max standard size (to be accepted into the orphanage)
417436
for large_orphan in large_orphans:
418437
assert_greater_than_or_equal(100000, large_orphan.get_vsize())
@@ -422,11 +441,10 @@ def test_orphanage_dos_large(self):
422441
assert not testres[0]["allowed"]
423442
assert_equal(testres[0]["reject-reason"], "missing-inputs")
424443

425-
num_individual_dosers = 30
426444
self.log.info(f"Connect {num_individual_dosers} peers and send a very large orphan from each one")
427445
# This test assumes that unrequested transactions are processed (skipping inv and
428446
# getdata steps because they require going through request delays)
429-
# Connect 20 peers and have each of them send a large orphan.
447+
# Connect 10 peers and have each of them send a large orphan.
430448
for large_orphan in large_orphans[:num_individual_dosers]:
431449
peer_doser_individual = node.add_p2p_connection(P2PInterface())
432450
peer_doser_individual.send_and_ping(msg_tx(large_orphan))
@@ -462,7 +480,7 @@ def test_orphanage_dos_large(self):
462480
peer_normal.wait_for_getdata([parent_txid_int])
463481

464482
self.log.info("Send another round of very large orphans from a DoSy peer")
465-
for large_orphan in large_orphans[30:]:
483+
for large_orphan in large_orphans[num_individual_dosers:]:
466484
peer_doser.send_and_ping(msg_tx(large_orphan))
467485

468486
# Something was evicted; the orphanage does not contain all large orphans + the 1p1c child
@@ -480,12 +498,12 @@ def test_orphanage_dos_many(self):
480498

481499
peer_normal = node.add_p2p_connection(P2PInterface())
482500

483-
# 2 sets of peers: the first set all send the same batch_size orphans. The second set each
484-
# sends batch_size distinct orphans.
501+
# The first set of peers all send the same batch_size orphans. Then a single peer sends
502+
# batch_single_doser distinct orphans.
485503
batch_size = 51
486504
num_peers_shared = 60
487-
num_peers_unique = 40
488-
505+
batch_single_doser = 100
506+
assert_greater_than(num_peers_shared * batch_size + batch_single_doser, 3000)
489507
# 60 peers * 51 orphans = 3060 announcements
490508
shared_orphans = [self.create_small_orphan() for _ in range(batch_size)]
491509
self.log.info(f"Send the same {batch_size} orphans from {num_peers_shared} DoSy peers (may take a while)")
@@ -525,22 +543,15 @@ def test_orphanage_dos_many(self):
525543
node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
526544
peer_normal.wait_for_getdata([parent_txid_int])
527545

528-
# Each of the num_peers_unique peers creates a distinct set of orphans
529-
many_orphans = [self.create_small_orphan() for _ in range(batch_size * num_peers_unique)]
530-
531-
self.log.info(f"Send sets of {batch_size} orphans from {num_peers_unique} DoSy peers (may take a while)")
532-
for peernum in range(num_peers_unique):
533-
peer_doser_batch = node.add_p2p_connection(P2PInterface())
534-
this_batch_orphans = many_orphans[batch_size*peernum : batch_size*(peernum+1)]
535-
for tx in this_batch_orphans:
536-
# Don't wait for responses, because it dramatically increases the runtime of this test.
537-
peer_doser_batch.send_without_ping(msg_tx(tx))
538-
539-
# Ensure at least one of the peer's orphans shows up in getorphantxs. Since each peer is
540-
# reserved a portion of orphanage space, this must happen as long as the orphans are not
541-
# rejected for some other reason.
542-
peer_doser_batch.sync_with_ping()
543-
self.wait_until(lambda: any([tx.txid_hex in node.getorphantxs() for tx in this_batch_orphans]))
546+
self.log.info(f"Send {batch_single_doser} new orphans from one DoSy peer")
547+
peer_doser_batch = node.add_p2p_connection(P2PInterface())
548+
this_batch_orphans = [self.create_small_orphan() for _ in range(batch_single_doser)]
549+
for tx in this_batch_orphans:
550+
# Don't wait for responses, because it dramatically increases the runtime of this test.
551+
peer_doser_batch.send_without_ping(msg_tx(tx))
552+
553+
peer_doser_batch.sync_with_ping()
554+
self.wait_until(lambda: any([tx.txid_hex in node.getorphantxs() for tx in this_batch_orphans]))
544555

545556
self.log.info("Check that orphan from normal peer still exists in orphanage")
546557
assert high_fee_child["txid"] in node.getorphantxs()

0 commit comments

Comments
 (0)