From f68700908e4faafb97ede5480aad1abba32d6dfd Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 25 Jan 2025 11:18:11 +1030 Subject: [PATCH] pytest: make test_gossip_throttle more reliable. By having gossipwith filter out messages we don't want, we can get the counts of expected messages correct, and not hit errors like this: ``` def test_gossip_throttle(node_factory, bitcoind, chainparams): """Make some gossip, test it gets throttled""" l1, l2, l3, l4 = node_factory.line_graph(4, wait_for_announce=True, opts=[{}, {}, {}, {'dev-throttle-gossip': None}]) # We expect: self-advertizement (3 messages for l1 and l4) plus # 4 node announcements, 3 channel announcements and 6 channel updates. # We also expect it to send a timestamp filter message. # (We won't take long enough to get a ping!) expected = 4 + 4 + 3 + 6 + 1 # l1 is unlimited start_fast = time.time() out1 = subprocess.run(['devtools/gossipwith', '--all-gossip', '--hex', '--network={}'.format(TEST_NETWORK), '--max-messages={}'.format(expected), '{}@localhost:{}'.format(l1.info['id'], l1.port)], check=True, timeout=TIMEOUT, stdout=subprocess.PIPE).stdout.split() time_fast = time.time() - start_fast assert time_fast < 2 # Remove timestamp filter, since timestamp will change! out1 = [m for m in out1 if not m.startswith(b'0109')] # l4 is throttled start_slow = time.time() out2 = subprocess.run(['devtools/gossipwith', '--all-gossip', '--hex', '--network={}'.format(TEST_NETWORK), '--max-messages={}'.format(expected), '{}@localhost:{}'.format(l4.info['id'], l4.port)], check=True, timeout=TIMEOUT, stdout=subprocess.PIPE).stdout.split() time_slow = time.time() - start_slow assert time_slow > 3 # Remove timestamp filter, since timestamp will change! out2 = [m for m in out2 if not m.startswith(b'0109')] # Contents should be identical (once uniquified, since each # doubles-up on its own gossip) > assert set(out1) == set(out2) E AssertionError: assert {b'010054b1907bdf639c9060e0fa4bca02419c46f75a99f0908b87a2e09711d5d031ba76b8fd07acc8be1b2fac9e31efb808e5d362c32ef4665... E Extra items in the left set: E b'01010ad5be8b9ba029245c2ae2d667af7ead7c0129c479c7fd7145a9b65931e90222082e6e4ab37ef60ebd10f1493d73e8bf7a40c4ae5f7d87cc...8488830b60f7e744ed9235eb0b1ba93283b315c035180266e44a554e494f524245414d2d333930353033622d6d6f64646564000000000000000000' E Extra items in the right set: E b'01079f87eb580b9e5f11dc211e9fb66abb3699999044f8fe146801162393364286c6000000010000006c010101' ``` Signed-off-by: Rusty Russell --- tests/test_gossip.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 700db4296..6ba70e4f7 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -2058,9 +2058,7 @@ def test_gossip_throttle(node_factory, bitcoind, chainparams): # We expect: self-advertizement (3 messages for l1 and l4) plus # 4 node announcements, 3 channel announcements and 6 channel updates. - # We also expect it to send a timestamp filter message. - # (We won't take long enough to get a ping!) - expected = 4 + 4 + 3 + 6 + 1 + expected = 4 + 4 + 3 + 6 # l1 is unlimited start_fast = time.time() @@ -2068,14 +2066,13 @@ def test_gossip_throttle(node_factory, bitcoind, chainparams): '--all-gossip', '--hex', '--network={}'.format(TEST_NETWORK), + '--filter=256,257,258', '--max-messages={}'.format(expected), '{}@localhost:{}'.format(l1.info['id'], l1.port)], check=True, timeout=TIMEOUT, stdout=subprocess.PIPE).stdout.split() time_fast = time.time() - start_fast assert time_fast < 2 - # Remove timestamp filter, since timestamp will change! - out1 = [m for m in out1 if not m.startswith(b'0109')] # l4 is throttled start_slow = time.time() @@ -2083,6 +2080,7 @@ def test_gossip_throttle(node_factory, bitcoind, chainparams): '--all-gossip', '--hex', '--network={}'.format(TEST_NETWORK), + '--filter=256,257,258', '--max-messages={}'.format(expected), '{}@localhost:{}'.format(l4.info['id'], l4.port)], check=True, @@ -2090,9 +2088,6 @@ def test_gossip_throttle(node_factory, bitcoind, chainparams): time_slow = time.time() - start_slow assert time_slow > 3 - # Remove timestamp filter, since timestamp will change! - out2 = [m for m in out2 if not m.startswith(b'0109')] - # Contents should be identical (once uniquified, since each # doubles-up on its own gossip) assert set(out1) == set(out2) @@ -2121,15 +2116,14 @@ def test_gossip_throttle(node_factory, bitcoind, chainparams): '--no-gossip', '--hex', '--network={}'.format(TEST_NETWORK), - '--max-messages={}'.format(expected + 1), + '--filter=256,257,258', + '--max-messages={}'.format(expected), '{}@localhost:{}'.format(l1.info['id'], l1.port), query], check=True, timeout=TIMEOUT, stdout=subprocess.PIPE).stdout.split() time_fast = time.time() - start_fast assert time_fast < 2 - # Ignore gossip_timestamp_filter and reply_short_channel_ids_end - out3 = [m for m in out3 if not m.startswith(b'0109') and not m.startswith(b'0106')] assert set(out1) == set(out3) start_slow = time.time() @@ -2137,6 +2131,7 @@ def test_gossip_throttle(node_factory, bitcoind, chainparams): '--no-gossip', '--hex', '--network={}'.format(TEST_NETWORK), + '--filter=256,257,258', '--max-messages={}'.format(expected), '{}@localhost:{}'.format(l4.info['id'], l4.port), query], @@ -2144,7 +2139,6 @@ def test_gossip_throttle(node_factory, bitcoind, chainparams): timeout=TIMEOUT, stdout=subprocess.PIPE).stdout.split() time_slow = time.time() - start_slow assert time_slow > 3 - out4 = [m for m in out4 if not m.startswith(b'0109')] assert set(out2) == set(out4)