From d75d48cd018cd85cf1708c5a3eb0b3eb6221fe30 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 5 Aug 2020 03:58:51 +0930 Subject: [PATCH 01/10] pytest: speed up test_restart_many_payments. I thought this was timing out because I made it slow with the change to txprepare as a plugin. In fact, it was timing out because sometimes gossip comes so fast it gets suppressed and we never get the log messags. Still, before this it took 98 seconds under valgrind and 24 under non-valgrind, so it's an improvement to time as well as robustness. Signed-off-by: Rusty Russell --- tests/test_connection.py | 54 ++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index dab72e581413..314dd7b8f66e 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -2000,40 +2000,52 @@ def test_fulfill_incoming_first(node_factory, bitcoind): def test_restart_many_payments(node_factory, bitcoind): l1 = node_factory.get_node(may_reconnect=True) - # On my laptop, these take 74 seconds and 44 seconds (with restart commented out) + # On my laptop, these take 89 seconds and 12 seconds if VALGRIND: num = 2 else: num = 5 + nodes = node_factory.get_nodes(num * 2, opts={'may_reconnect': True}) + innodes = nodes[:num] + outnodes = nodes[num:] + + # Fund up-front to save some time. + dests = {l1.rpc.newaddr()['bech32']: (10**6 + 1000000) / 10**8 * num} + for n in innodes: + dests[n.rpc.newaddr()['bech32']] = (10**6 + 1000000) / 10**8 + bitcoind.rpc.sendmany("", dests) + bitcoind.generate_block(1) + sync_blockheight(bitcoind, [l1] + innodes) + # Nodes with channels into the main node - innodes = node_factory.get_nodes(num, opts={'may_reconnect': True}) - inchans = [] for n in innodes: n.rpc.connect(l1.info['id'], 'localhost', l1.port) - inchans.append(n.fund_channel(l1, 10**6, False)) + n.rpc.fundchannel(l1.info['id'], 10**6) # Nodes with channels out of the main node - outnodes = node_factory.get_nodes(len(innodes), opts={'may_reconnect': True}) - outchans = [] for n in outnodes: - n.rpc.connect(l1.info['id'], 'localhost', l1.port) - outchans.append(l1.fund_channel(n, 10**6, False)) + l1.rpc.connect(n.info['id'], 'localhost', n.port) + # OK to use change from previous fundings + l1.rpc.fundchannel(n.info['id'], 10**6, minconf=0) - # Make sure they're all announced. - bitcoind.generate_block(5) + # Now mine them, get scids. + bitcoind.generate_block(6, wait_for_mempool=num * 2) + sync_blockheight(bitcoind, [l1] + nodes) + + wait_for(lambda: [only_one(n.rpc.listpeers()['peers'])['channels'][0]['state'] for n in nodes] == ['CHANNELD_NORMAL'] * len(nodes)) + + inchans = [] + for n in innodes: + inchans.append(only_one(n.rpc.listpeers()['peers'])['channels'][0]['short_channel_id']) + + outchans = [] + for n in outnodes: + outchans.append(only_one(n.rpc.listpeers()['peers'])['channels'][0]['short_channel_id']) - # We wait for each node to see each dir active, and its own - # channel CHANNELD_NORMAL - logs = ([r'update for channel {}/0 now ACTIVE'.format(scid) - for scid in inchans + outchans] - + [r'update for channel {}/1 now ACTIVE'.format(scid) - for scid in inchans + outchans] - + ['to CHANNELD_NORMAL']) - - # Now do all the waiting at once: if !DEVELOPER, this can be *very* slow! - for n in innodes + outnodes: - n.daemon.wait_for_logs(logs) + # Now make sure every node sees every channel. + for n in nodes + [l1]: + wait_for(lambda: [c['public'] for c in n.rpc.listchannels()['channels']] == [True] * len(nodes) * 2) # Manually create routes, get invoices Payment = namedtuple('Payment', ['innode', 'route', 'payment_hash']) From 0bd1cf935b70710bf3bc6df32c7eac52eca66baf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 5 Aug 2020 03:59:51 +0930 Subject: [PATCH 02/10] pytest: new join_nodes to allow you to get all the nodes then join some of them. This lets you get_nodes() and join some later. Signed-off-by: Rusty Russell --- contrib/pyln-testing/pyln/testing/utils.py | 30 ++++++++++++---------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index bbc7e80c97d3..a12a41ac3906 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -1072,13 +1072,10 @@ def get_node(self, node_id=None, options=None, dbfile=None, raise return node - def line_graph(self, num_nodes, fundchannel=True, fundamount=10**6, wait_for_announce=False, opts=None, announce_channels=True): - """ Create nodes, connect them and optionally fund channels. - """ + def join_nodes(self, nodes, fundchannel=True, fundamount=10**6, wait_for_announce=False, announce_channels=True) -> None: + """Given nodes, connect them in a line, optionally funding a channel""" assert not (wait_for_announce and not announce_channels), "You've asked to wait for an announcement that's not coming. (wait_for_announce=True,announce_channels=False)" - nodes = self.get_nodes(num_nodes, opts=opts) - bitcoin = nodes[0].bitcoin - connections = [(nodes[i], nodes[i + 1]) for i in range(0, num_nodes - 1)] + connections = [(nodes[i], nodes[i + 1]) for i in range(len(nodes) - 1)] for src, dst in connections: src.rpc.connect(dst.info['id'], 'localhost', dst.port) @@ -1088,21 +1085,22 @@ def line_graph(self, num_nodes, fundchannel=True, fundamount=10**6, wait_for_ann if not fundchannel: for src, dst in connections: dst.daemon.wait_for_log(r'{}-.*openingd-chan#[0-9]*: Handed peer, entering loop'.format(src.info['id'])) - return nodes + return + bitcoind = nodes[0].bitcoin # If we got here, we want to fund channels for src, dst in connections: addr = src.rpc.newaddr()['bech32'] - src.bitcoin.rpc.sendtoaddress(addr, (fundamount + 1000000) / 10**8) + bitcoind.rpc.sendtoaddress(addr, (fundamount + 1000000) / 10**8) - bitcoin.generate_block(1) + bitcoind.generate_block(1) for src, dst in connections: wait_for(lambda: len(src.rpc.listfunds()['outputs']) > 0) tx = src.rpc.fundchannel(dst.info['id'], fundamount, announce=announce_channels) - wait_for(lambda: tx['txid'] in bitcoin.rpc.getrawmempool()) + wait_for(lambda: tx['txid'] in bitcoind.rpc.getrawmempool()) # Confirm all channels and wait for them to become usable - bitcoin.generate_block(1) + bitcoind.generate_block(1) scids = [] for src, dst in connections: wait_for(lambda: src.channel_state(dst) == 'CHANNELD_NORMAL') @@ -1111,9 +1109,9 @@ def line_graph(self, num_nodes, fundchannel=True, fundamount=10**6, wait_for_ann scids.append(scid) if not wait_for_announce: - return nodes + return - bitcoin.generate_block(5) + bitcoind.generate_block(5) def both_dirs_ready(n, scid): resp = n.rpc.listchannels(scid) @@ -1129,6 +1127,12 @@ def both_dirs_ready(n, scid): for end in (nodes[0], nodes[-1]): wait_for(lambda: 'alias' in only_one(end.rpc.listnodes(n.info['id'])['nodes'])) + def line_graph(self, num_nodes, fundchannel=True, fundamount=10**6, wait_for_announce=False, opts=None, announce_channels=True): + """ Create nodes, connect them and optionally fund channels. + """ + nodes = self.get_nodes(num_nodes, opts=opts) + + self.join_nodes(nodes, fundchannel, fundamount, wait_for_announce, announce_channels) return nodes def killall(self, expected_successes): From c7f06f075a5129591d262cbf1eb0e18cf8d29641 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 5 Aug 2020 04:00:51 +0930 Subject: [PATCH 03/10] pytest: make join_nodes / line_graph wait for updates in both dirs. This is what fund_channel() does, which is more thorough than what we were doing. But since the order of the logs is undefined, we need to be a little careful. Signed-off-by: Rusty Russell --- contrib/pyln-testing/pyln/testing/utils.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index a12a41ac3906..cfabb9cde09a 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -1105,9 +1105,29 @@ def join_nodes(self, nodes, fundchannel=True, fundamount=10**6, wait_for_announc for src, dst in connections: wait_for(lambda: src.channel_state(dst) == 'CHANNELD_NORMAL') scid = src.get_channel_scid(dst) - src.daemon.wait_for_log(r'Received channel_update for channel {scid}/. now ACTIVE'.format(scid=scid)) scids.append(scid) + # We don't want to assume message order here. + # Wait for ends: + nodes[0].daemon.wait_for_logs([r'update for channel {}/0 now ACTIVE' + .format(scids[0]), + r'update for channel {}/1 now ACTIVE' + .format(scids[0])]) + nodes[-1].daemon.wait_for_logs([r'update for channel {}/0 now ACTIVE' + .format(scids[-1]), + r'update for channel {}/1 now ACTIVE' + .format(scids[-1])]) + # Now wait for intermediate nodes: + for i, n in enumerate(nodes[1:-1]): + n.daemon.wait_for_logs([r'update for channel {}/0 now ACTIVE' + .format(scids[i]), + r'update for channel {}/1 now ACTIVE' + .format(scids[i]), + r'update for channel {}/0 now ACTIVE' + .format(scids[i + 1]), + r'update for channel {}/1 now ACTIVE' + .format(scids[i + 1])]) + if not wait_for_announce: return From 351db81eb17b3a025987c4644d571c1885f4e87f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 5 Aug 2020 04:01:51 +0930 Subject: [PATCH 04/10] pytest: fix assumption in test_onchain_their_unilateral_out In fact, the 5 blocks generate above were not seen by nodes until later, making me very confused. Signed-off-by: Rusty Russell --- tests/test_closing.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_closing.py b/tests/test_closing.py index a6ff6f18ac89..b57cad250d18 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -1519,6 +1519,7 @@ def test_onchain_their_unilateral_out(node_factory, bitcoind): bitcoind.generate_block(5) l1.wait_channel_active(c12) + sync_blockheight(bitcoind, [l1, l2]) route = l2.rpc.getroute(l1.info['id'], 10**8, 1)["route"] assert len(route) == 1 @@ -1545,8 +1546,8 @@ def try_pay(): l1.daemon.wait_for_log(' to ONCHAIN') l2.daemon.wait_for_log('THEIR_UNILATERAL/OUR_HTLC') - # l2 should wait til to_self_delay (6), then fulfill onchain - l1.bitcoin.generate_block(5) + # l2 should wait til to_self_delay (10), then fulfill onchain + l1.bitcoin.generate_block(9) l2.wait_for_onchaind_broadcast('OUR_HTLC_TIMEOUT_TO_US', 'THEIR_UNILATERAL/OUR_HTLC') From 7d279534ca744ecafaa5a7dbc4b2032e33bb45e7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 7 Aug 2020 12:39:37 +0930 Subject: [PATCH 05/10] pytest: fix flake in test_channel_opened_notification We can have the message before the node ready message which line_graph waits for. Signed-off-by: Rusty Russell --- tests/test_plugin.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index a1830c9a773f..ae6eecee24f4 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -774,6 +774,9 @@ def test_channel_opened_notification(node_factory): amount = 10**6 l1, l2 = node_factory.line_graph(2, fundchannel=True, fundamount=amount, opts=opts) + + # Might have already passed, so reset start. + l2.daemon.logsearch_start = 0 l2.daemon.wait_for_log(r"A channel was opened to us by {}, " "with an amount of {}*" .format(l1.info["id"], amount)) From 15dcf1bf26ce5608395b95cbe7ad777346c16288 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 7 Aug 2020 12:43:55 +0930 Subject: [PATCH 06/10] pytest: make sure all nodes see funds using sync_blockheight We might have funds prior to calling join_nodes(), so testing that we've all seen the block is better. Signed-off-by: Rusty Russell --- contrib/pyln-testing/pyln/testing/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index cfabb9cde09a..a9616bea1b89 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -1094,8 +1094,8 @@ def join_nodes(self, nodes, fundchannel=True, fundamount=10**6, wait_for_announc bitcoind.rpc.sendtoaddress(addr, (fundamount + 1000000) / 10**8) bitcoind.generate_block(1) + sync_blockheight(bitcoind, nodes) for src, dst in connections: - wait_for(lambda: len(src.rpc.listfunds()['outputs']) > 0) tx = src.rpc.fundchannel(dst.info['id'], fundamount, announce=announce_channels) wait_for(lambda: tx['txid'] in bitcoind.rpc.getrawmempool()) From c647d45170e75b895a541a1f61cb4378b939134e Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 7 Aug 2020 12:44:55 +0930 Subject: [PATCH 07/10] pytest: use get_nodes more widely. I started replacing all get_node() calls, but got bored, so then just did the tests which call get_node() 3 times or more. Ends up not making a measurable speed difference, but it does make some things neater and more standard. Times with SLOW_MACHINE=1 (given that's how Travis tests): Time before (non-valgrind): 393 sec (had 3 failures?) Time after (non-valgrind): 410 sec Time before (valgrind): 890 seconds (had 2 failures) Time after (valgrind): 892 sec Signed-off-by: Rusty Russell --- tests/test_closing.py | 272 ++++++++++++++++----------------------- tests/test_connection.py | 33 +++-- tests/test_gossip.py | 50 +++---- tests/test_invoices.py | 4 +- tests/test_misc.py | 56 ++++---- tests/test_pay.py | 80 +++++------- tests/test_plugin.py | 11 +- 7 files changed, 216 insertions(+), 290 deletions(-) diff --git a/tests/test_closing.py b/tests/test_closing.py index b57cad250d18..193f2471fc04 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -278,16 +278,14 @@ def test_closing_negotiation_reconnect(node_factory, bitcoind): disconnects = ['-WIRE_CLOSING_SIGNED', '@WIRE_CLOSING_SIGNED', '+WIRE_CLOSING_SIGNED'] - l1 = node_factory.get_node(disconnect=disconnects, may_reconnect=True) - l2 = node_factory.get_node(may_reconnect=True) - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - - chan = l1.fund_channel(l2, 10**6) + l1, l2 = node_factory.line_graph(2, opts=[{'disconnect': disconnects, + 'may_reconnect': True}, + {'may_reconnect': True}]) l1.pay(l2, 200000000) assert bitcoind.rpc.getmempoolinfo()['size'] == 0 - l1.rpc.close(chan) + l1.rpc.close(l2.info['id']) l1.daemon.wait_for_log(' to CHANNELD_SHUTTING_DOWN') l2.daemon.wait_for_log(' to CHANNELD_SHUTTING_DOWN') @@ -362,20 +360,14 @@ def test_closing_specified_destination(node_factory, bitcoind, chainparams): def closing_negotiation_step(node_factory, bitcoind, chainparams, opts): - rate = 29006 # closing fee negotiation starts at 21000 - opener = node_factory.get_node(feerates=(rate, rate, rate, rate)) - - rate = 27625 # closing fee negotiation starts at 20000 - peer = node_factory.get_node(feerates=(rate, rate, rate, rate)) + orate = 29006 # closing fee negotiation starts at 21000 + prate = 27625 # closing fee negotiation starts at 20000 + opener, peer = node_factory.line_graph(2, opts=[{'feerates': (orate, orate, orate, orate)}, + {'feerates': (prate, prate, prate, prate)}]) opener_id = opener.info['id'] peer_id = peer.info['id'] - fund_amount = 10**6 - - opener.rpc.connect(peer_id, 'localhost', peer.port) - opener.fund_channel(peer, fund_amount) - assert bitcoind.rpc.getmempoolinfo()['size'] == 0 if opts['close_initiated_by'] == 'opener': @@ -508,15 +500,14 @@ def test_penalty_inhtlc(node_factory, bitcoind, executor, chainparams): coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') # We suppress each one after first commit; HTLC gets added not fulfilled. # Feerates identical so we don't get gratuitous commit to update them - l1 = node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED-nocommit'], - may_fail=True, feerates=(7500, 7500, 7500, 7500), - allow_broken_log=True, - options={'plugin': coin_mvt_plugin}) - l2 = node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED-nocommit'], - options={'plugin': coin_mvt_plugin}) + l1, l2 = node_factory.line_graph(2, opts=[{'disconnect': ['=WIRE_COMMITMENT_SIGNED-nocommit'], + 'may_fail': True, + 'feerates': (7500, 7500, 7500, 7500), + 'allow_broken_log': True, + 'plugin': coin_mvt_plugin}, + {'disconnect': ['=WIRE_COMMITMENT_SIGNED-nocommit'], + 'plugin': coin_mvt_plugin}]) - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l1.fund_channel(l2, 10**6) channel_id = first_channel_id(l1, l2) # Now, this will get stuck due to l1 commit being disabled.. @@ -605,16 +596,14 @@ def test_penalty_outhtlc(node_factory, bitcoind, executor, chainparams): coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') # First we need to get funds to l2, so suppress after second. # Feerates identical so we don't get gratuitous commit to update them - l1 = node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED*3-nocommit'], - may_fail=True, - feerates=(7500, 7500, 7500, 7500), - allow_broken_log=True, - options={'plugin': coin_mvt_plugin}) - l2 = node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED*3-nocommit'], - options={'plugin': coin_mvt_plugin}) - - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l1.fund_channel(l2, 10**6) + l1, l2 = node_factory.line_graph(2, + opts=[{'disconnect': ['=WIRE_COMMITMENT_SIGNED*3-nocommit'], + 'may_fail': True, + 'feerates': (7500, 7500, 7500, 7500), + 'allow_broken_log': True, + 'plugin': coin_mvt_plugin}, + {'disconnect': ['=WIRE_COMMITMENT_SIGNED*3-nocommit'], + 'plugin': coin_mvt_plugin}]) channel_id = first_channel_id(l1, l2) # Move some across to l2. @@ -727,38 +716,29 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams): # We track channel balances, to verify that accounting is ok. coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') - l1 = node_factory.get_node(disconnect=['=WIRE_UPDATE_FULFILL_HTLC', - '-WIRE_UPDATE_FULFILL_HTLC'], - may_reconnect=True, - options={'dev-no-reconnect': None}) - l2 = node_factory.get_node(options={'plugin': coin_mvt_plugin, - 'disable-mpp': None, - 'dev-no-reconnect': None}, - may_reconnect=True, - allow_broken_log=True) - l3 = node_factory.get_node(options={'plugin': coin_mvt_plugin, - 'dev-no-reconnect': None}, - may_reconnect=True, - allow_broken_log=True) - l4 = node_factory.get_node(may_reconnect=True, options={'dev-no-reconnect': None}) - - l2.rpc.connect(l1.info['id'], 'localhost', l1.port) - l2.rpc.connect(l3.info['id'], 'localhost', l3.port) - l3.rpc.connect(l4.info['id'], 'localhost', l4.port) + l1, l2, l3, l4 = node_factory.line_graph(4, + opts=[{'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC'], + 'may_reconnect': True, + 'dev-no-reconnect': None}, + {'plugin': coin_mvt_plugin, + 'disable-mpp': None, + 'dev-no-reconnect': None, + 'may_reconnect': True, + 'allow_broken_log': True}, + {'plugin': coin_mvt_plugin, + 'dev-no-reconnect': None, + 'may_reconnect': True, + 'allow_broken_log': True}, + {'dev-no-reconnect': None, + 'may_reconnect': True}], + wait_for_announce=True) - c12 = l2.fund_channel(l1, 10**6) - l2.fund_channel(l3, 10**6) - c34 = l3.fund_channel(l4, 10**6) channel_id = first_channel_id(l2, l3) - bitcoind.generate_block(5) - l1.wait_channel_active(c34) - l4.wait_channel_active(c12) - # push some money so that 1 + 4 can both send htlcs - inv = l1.rpc.invoice(10**9 // 2, '1', 'balancer') - l2.rpc.pay(inv['bolt11']) - l2.rpc.waitsendpay(inv['payment_hash']) + inv = l2.rpc.invoice(10**9 // 2, '1', 'balancer') + l1.rpc.pay(inv['bolt11']) + l1.rpc.waitsendpay(inv['payment_hash']) inv = l4.rpc.invoice(10**9 // 2, '1', 'balancer') l2.rpc.pay(inv['bolt11']) @@ -874,43 +854,31 @@ def test_penalty_htlc_tx_timeout(node_factory, bitcoind, chainparams): # We track channel balances, to verify that accounting is ok. coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') - l1 = node_factory.get_node(disconnect=['=WIRE_UPDATE_FULFILL_HTLC', - '-WIRE_UPDATE_FULFILL_HTLC'], - may_reconnect=True, - options={'dev-no-reconnect': None}) - l2 = node_factory.get_node(options={'plugin': coin_mvt_plugin, - 'dev-no-reconnect': None}, - may_reconnect=True, - allow_broken_log=True) - l3 = node_factory.get_node(options={'plugin': coin_mvt_plugin, - 'dev-no-reconnect': None}, - may_reconnect=True, - allow_broken_log=True) - l4 = node_factory.get_node(may_reconnect=True, options={'dev-no-reconnect': None}) - l5 = node_factory.get_node(disconnect=['-WIRE_UPDATE_FULFILL_HTLC'], - may_reconnect=True, - options={'dev-no-reconnect': None}) - - l2.rpc.connect(l1.info['id'], 'localhost', l1.port) - l2.rpc.connect(l3.info['id'], 'localhost', l3.port) - l3.rpc.connect(l4.info['id'], 'localhost', l4.port) - l3.rpc.connect(l5.info['id'], 'localhost', l5.port) + l1, l2, l3, l4, l5 = node_factory.get_nodes(5, + opts=[{'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC'], + 'may_reconnect': True, + 'dev-no-reconnect': None}, + {'plugin': coin_mvt_plugin, + 'dev-no-reconnect': None, + 'may_reconnect': True, + 'allow_broken_log': True}, + {'plugin': coin_mvt_plugin, + 'dev-no-reconnect': None, + 'may_reconnect': True, + 'allow_broken_log': True}, + {'dev-no-reconnect': None}, + {'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC'], + 'may_reconnect': True, + 'dev-no-reconnect': None}]) + + node_factory.join_nodes([l1, l2, l3, l4], wait_for_announce=True) + node_factory.join_nodes([l3, l5], wait_for_announce=True) - c12 = l2.fund_channel(l1, 10**6) - l2.fund_channel(l3, 10**6) - c34 = l3.fund_channel(l4, 10**6) - c35 = l3.fund_channel(l5, 10**6) channel_id = first_channel_id(l2, l3) - bitcoind.generate_block(5) - l1.wait_channel_active(c34) - l1.wait_channel_active(c35) - l4.wait_channel_active(c12) - l5.wait_channel_active(c12) - # push some money so that 1 + 4 can both send htlcs - inv = l1.rpc.invoice(10**9 // 2, '1', 'balancer') - l2.rpc.pay(inv['bolt11']) + inv = l2.rpc.invoice(10**9 // 2, '1', 'balancer') + l1.rpc.pay(inv['bolt11']) inv = l4.rpc.invoice(10**9 // 2, '1', 'balancer') l2.rpc.pay(inv['bolt11']) @@ -1027,22 +995,22 @@ def test_onchain_first_commit(node_factory, bitcoind): # HTLC 1->2, 1 fails just after funding. disconnects = ['+WIRE_FUNDING_LOCKED', 'permfail'] - l1 = node_factory.get_node(disconnect=disconnects, options={'plugin': coin_mvt_plugin}) # Make locktime different, as we once had them reversed! - l2 = node_factory.get_node(options={'watchtime-blocks': 10, 'plugin': coin_mvt_plugin}) + l1, l2 = node_factory.line_graph(2, opts=[{'disconnect': disconnects, + 'plugin': coin_mvt_plugin}, + {'watchtime-blocks': 10, + 'plugin': coin_mvt_plugin}], + fundchannel=False) l1.fundwallet(10**7) - - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l1.rpc.fundchannel(l2.info['id'], 10**6) l1.daemon.wait_for_log('sendrawtx exit 0') - l1.bitcoin.generate_block(1) + bitcoind.generate_block(1) # l1 will drop to chain. l1.daemon.wait_for_log('permfail') l1.daemon.wait_for_log('sendrawtx exit 0') - l1.bitcoin.generate_block(1) + bitcoind.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') l2.daemon.wait_for_log(' to ONCHAIN') @@ -1117,14 +1085,11 @@ def test_onchain_unwatch(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") def test_onchaind_replay(node_factory, bitcoind): disconnects = ['+WIRE_REVOKE_AND_ACK', 'permfail'] - options = {'watchtime-blocks': 201, 'cltv-delta': 101} # Feerates identical so we don't get gratuitous commit to update them - l1 = node_factory.get_node(options=options, disconnect=disconnects, - feerates=(7500, 7500, 7500, 7500)) - l2 = node_factory.get_node(options=options) - - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l1.fund_channel(l2, 10**6) + l1, l2 = node_factory.line_graph(2, opts=[{'watchtime-blocks': 201, 'cltv-delta': 101, + 'disconnect': disconnects, + 'feerates': (7500, 7500, 7500, 7500)}, + {'watchtime-blocks': 201, 'cltv-delta': 101}]) rhash = l2.rpc.invoice(10**8, 'onchaind_replay', 'desc')['payment_hash'] routestep = { @@ -1176,13 +1141,12 @@ def test_onchain_dust_out(node_factory, bitcoind, executor): # HTLC 1->2, 1 fails after it's irrevocably committed disconnects = ['@WIRE_REVOKE_AND_ACK', 'permfail'] # Feerates identical so we don't get gratuitous commit to update them - l1 = node_factory.get_node(disconnect=disconnects, - feerates=(7500, 7500, 7500, 7500), - options={'plugin': coin_mvt_plugin}) - l2 = node_factory.get_node(options={'plugin': coin_mvt_plugin}) + l1, l2 = node_factory.line_graph(2, + opts=[{'disconnect': disconnects, + 'feerates': (7500, 7500, 7500, 7500), + 'plugin': coin_mvt_plugin}, + {'plugin': coin_mvt_plugin}]) - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l1.fund_channel(l2, 10**6) channel_id = first_channel_id(l1, l2) # Must be dust! @@ -1248,13 +1212,12 @@ def test_onchain_timeout(node_factory, bitcoind, executor): # HTLC 1->2, 1 fails just after it's irrevocably committed disconnects = ['+WIRE_REVOKE_AND_ACK*3', 'permfail'] # Feerates identical so we don't get gratuitous commit to update them - l1 = node_factory.get_node(disconnect=disconnects, - feerates=(7500, 7500, 7500, 7500), - options={'plugin': coin_mvt_plugin}) - l2 = node_factory.get_node(options={'plugin': coin_mvt_plugin}) + l1, l2 = node_factory.line_graph(2, + opts=[{'disconnect': disconnects, + 'feerates': (7500, 7500, 7500, 7500), + 'plugin': coin_mvt_plugin}, + {'plugin': coin_mvt_plugin}]) - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l1.fund_channel(l2, 10**6) channel_id = first_channel_id(l1, l2) rhash = l2.rpc.invoice(10**8, 'onchain_timeout', 'desc')['payment_hash'] @@ -1333,9 +1296,10 @@ def test_onchain_middleman(node_factory, bitcoind): # HTLC 1->2->3, 1->2 goes down after 2 gets preimage from 3. disconnects = ['-WIRE_UPDATE_FULFILL_HTLC', 'permfail'] - l1 = node_factory.get_node(options={'plugin': coin_mvt_plugin}) - l2 = node_factory.get_node(disconnect=disconnects, options={'plugin': coin_mvt_plugin}) - l3 = node_factory.get_node() + l1, l2, l3 = node_factory.get_nodes(3, opts=[{'plugin': coin_mvt_plugin}, + {'plugin': coin_mvt_plugin, + 'disconnect': disconnects}, + {}]) # l2 connects to both, so l1 can't reconnect and thus l2 drops to chain l2.rpc.connect(l1.info['id'], 'localhost', l1.port) @@ -1422,12 +1386,11 @@ def test_onchain_middleman_their_unilateral_in(node_factory, bitcoind): l1_disconnects = ['=WIRE_UPDATE_FULFILL_HTLC', 'permfail'] l2_disconnects = ['-WIRE_UPDATE_FULFILL_HTLC'] - l1 = node_factory.get_node(disconnect=l1_disconnects, - options={'plugin': coin_mvt_plugin}) - l2 = node_factory.get_node(disconnect=l2_disconnects, - options={'plugin': coin_mvt_plugin}) - l3 = node_factory.get_node() - + l1, l2, l3 = node_factory.get_nodes(3, opts=[{'plugin': coin_mvt_plugin, + 'disconnect': l1_disconnects}, + {'plugin': coin_mvt_plugin, + 'disconnect': l2_disconnects}, + {}]) l2.rpc.connect(l1.info['id'], 'localhost', l1.port) l2.rpc.connect(l3.info['id'], 'localhost', l3.port) @@ -1508,20 +1471,12 @@ def test_onchain_their_unilateral_out(node_factory, bitcoind): disconnects = ['-WIRE_UPDATE_FAIL_HTLC', 'permfail'] - l1 = node_factory.get_node(disconnect=disconnects, - options={'plugin': coin_mvt_plugin}) - l2 = node_factory.get_node(options={'plugin': coin_mvt_plugin}) - - l2.rpc.connect(l1.info['id'], 'localhost', l1.port) - - c12 = l2.fund_channel(l1, 10**6) + l1, l2 = node_factory.line_graph(2, opts=[{'plugin': coin_mvt_plugin}, + {'disconnect': disconnects, + 'plugin': coin_mvt_plugin}]) channel_id = first_channel_id(l1, l2) - bitcoind.generate_block(5) - l1.wait_channel_active(c12) - sync_blockheight(bitcoind, [l1, l2]) - - route = l2.rpc.getroute(l1.info['id'], 10**8, 1)["route"] + route = l1.rpc.getroute(l2.info['id'], 10**8, 1)["route"] assert len(route) == 1 q = queue.Queue() @@ -1530,7 +1485,7 @@ def try_pay(): try: # rhash is fake rhash = 'B1' * 32 - l2.rpc.sendpay(route, rhash) + l1.rpc.sendpay(route, rhash) q.put(None) except Exception as err: q.put(err) @@ -1539,16 +1494,16 @@ def try_pay(): t.daemon = True t.start() - # l1 will drop to chain. - l1.daemon.wait_for_log('sendrawtx exit 0') - l1.bitcoin.generate_block(1) - l2.daemon.wait_for_log(' to ONCHAIN') + # l2 will drop to chain. + l2.daemon.wait_for_log('sendrawtx exit 0') + l2.bitcoin.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') - l2.daemon.wait_for_log('THEIR_UNILATERAL/OUR_HTLC') + l2.daemon.wait_for_log(' to ONCHAIN') + l1.daemon.wait_for_log('THEIR_UNILATERAL/OUR_HTLC') - # l2 should wait til to_self_delay (10), then fulfill onchain - l1.bitcoin.generate_block(9) - l2.wait_for_onchaind_broadcast('OUR_HTLC_TIMEOUT_TO_US', + # l1 should wait til to_self_delay (10), then fulfill onchain + l2.bitcoin.generate_block(9) + l1.wait_for_onchaind_broadcast('OUR_HTLC_TIMEOUT_TO_US', 'THEIR_UNILATERAL/OUR_HTLC') err = q.get(timeout=10) @@ -1559,13 +1514,13 @@ def try_pay(): assert not t.is_alive() # 100 blocks after last spend, l1+l2 should be done. - l1.bitcoin.generate_block(100) - l2.daemon.wait_for_log('onchaind complete, forgetting peer') + l2.bitcoin.generate_block(100) l1.daemon.wait_for_log('onchaind complete, forgetting peer') + l2.daemon.wait_for_log('onchaind complete, forgetting peer') # Verify accounting for l1 & l2 - assert account_balance(l1, channel_id) == 0 assert account_balance(l2, channel_id) == 0 + assert account_balance(l1, channel_id) == 0 def test_listfunds_after_their_unilateral(node_factory, bitcoind): @@ -1603,12 +1558,9 @@ def test_onchain_feechange(node_factory, bitcoind, executor): # We need 2 to drop to chain, because then 1's HTLC timeout tx # is generated on-the-fly, and is thus feerate sensitive. disconnects = ['-WIRE_UPDATE_FAIL_HTLC', 'permfail'] - l1 = node_factory.get_node(may_reconnect=True) - l2 = node_factory.get_node(disconnect=disconnects, - may_reconnect=True) - - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l1.fund_channel(l2, 10**6) + l1, l2 = node_factory.line_graph(2, opts=[{'may_reconnect': True}, + {'may_reconnect': True, + 'disconnect': disconnects}]) rhash = l2.rpc.invoice(10**8, 'onchain_timeout', 'desc')['payment_hash'] # We underpay, so it fails. diff --git a/tests/test_connection.py b/tests/test_connection.py index 314dd7b8f66e..d099706fb8f6 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -171,12 +171,11 @@ def test_opening_tiny_channel(node_factory): l4_min_capacity = 10000 # the current default l5_min_capacity = 20000 # a server with more than default minimum - l1 = node_factory.get_node(options={'min-capacity-sat': l1_min_capacity}) - l2 = node_factory.get_node(options={'min-capacity-sat': l2_min_capacity}) - l3 = node_factory.get_node(options={'min-capacity-sat': l3_min_capacity}) - l4 = node_factory.get_node(options={'min-capacity-sat': l4_min_capacity}) - l5 = node_factory.get_node(options={'min-capacity-sat': l5_min_capacity}) - + l1, l2, l3, l4, l5 = node_factory.get_nodes(5, opts=[{'min-capacity-sat': l1_min_capacity}, + {'min-capacity-sat': l2_min_capacity}, + {'min-capacity-sat': l3_min_capacity}, + {'min-capacity-sat': l4_min_capacity}, + {'min-capacity-sat': l5_min_capacity}]) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.rpc.connect(l3.info['id'], 'localhost', l3.port) l1.rpc.connect(l4.info['id'], 'localhost', l4.port) @@ -212,9 +211,7 @@ def test_opening_tiny_channel(node_factory): def test_second_channel(node_factory): - l1 = node_factory.get_node() - l2 = node_factory.get_node() - l3 = node_factory.get_node() + l1, l2, l3 = node_factory.get_nodes(3) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.rpc.connect(l3.info['id'], 'localhost', l3.port) @@ -428,9 +425,7 @@ def test_reconnect_no_update(node_factory, executor): def test_connect_stresstest(node_factory, executor): # This test is unreliable, but it's better than nothing. - l1 = node_factory.get_node(may_reconnect=True) - l2 = node_factory.get_node(may_reconnect=True) - l3 = node_factory.get_node(may_reconnect=True) + l1, l2, l3 = node_factory.get_nodes(3, opts={'may_reconnect': True}) # Hack l3 into a clone of l2, to stress reconnect code. l3.stop() @@ -1162,9 +1157,8 @@ def _fundchannel(l1, l2, amount, close_to): @unittest.skipIf(TEST_NETWORK != 'regtest', "External wallet support doesn't work with elements yet.") def test_funding_external_wallet(node_factory, bitcoind): - l1 = node_factory.get_node(options={'funding-confirms': 2}) - l2 = node_factory.get_node(options={'funding-confirms': 2}) - l3 = node_factory.get_node() + l1, l2, l3 = node_factory.get_nodes(3, opts=[{'funding-confirms': 2}, + {'funding-confirms': 2}, {}]) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) assert(l1.rpc.listpeers()['peers'][0]['id'] == l2.info['id']) @@ -1422,7 +1416,12 @@ def test_update_fee(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") def test_fee_limits(node_factory, bitcoind): - l1, l2 = node_factory.line_graph(2, opts={'dev-max-fee-multiplier': 5, 'may_reconnect': True}, fundchannel=True) + l1, l2, l3, l4 = node_factory.get_nodes(4, opts=[{'dev-max-fee-multiplier': 5, 'may_reconnect': True}, + {'dev-max-fee-multiplier': 5, 'may_reconnect': True}, + {'ignore-fee-limits': True, 'may_reconnect': True}, + {}]) + + node_factory.join_nodes([l1, l2], fundchannel=True) # Kick off fee adjustment using HTLC. l1.pay(l2, 1000) @@ -1441,7 +1440,6 @@ def test_fee_limits(node_factory, bitcoind): sync_blockheight(bitcoind, [l1, l2]) # Trying to open a channel with too low a fee-rate is denied - l4 = node_factory.get_node() l1.rpc.connect(l4.info['id'], 'localhost', l4.port) with pytest.raises(RpcError, match='They sent error .* feerate_per_kw 253 below minimum'): l1.fund_channel(l4, 10**6) @@ -1452,7 +1450,6 @@ def test_fee_limits(node_factory, bitcoind): l1.start() # Try with node which sets --ignore-fee-limits - l3 = node_factory.get_node(options={'ignore-fee-limits': 'true'}, may_reconnect=True) l1.rpc.connect(l3.info['id'], 'localhost', l3.port) chan = l1.fund_channel(l3, 10**6) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index fee9d9f16bc8..562056ce3827 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -456,10 +456,11 @@ def test_routing_gossip_reconnect(node_factory): # Connect two peers, reconnect and then see if we resume the # gossip. disconnects = ['-WIRE_CHANNEL_ANNOUNCEMENT'] - l1 = node_factory.get_node(disconnect=disconnects, - may_reconnect=True) - l2 = node_factory.get_node(may_reconnect=True) - l3 = node_factory.get_node() + l1, l2, l3 = node_factory.get_nodes(3, + opts=[{'disconnect': disconnects, + 'may_reconnect': True}, + {'may_reconnect': True}, + {}]) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.openchannel(l2, 20000) @@ -475,17 +476,13 @@ def test_routing_gossip_reconnect(node_factory): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") def test_gossip_no_empty_announcements(node_factory, bitcoind): # Need full IO logging so we can see gossip - opts = {'log-level': 'io'} - l1, l2 = node_factory.get_nodes(2, opts=opts) # l3 sends CHANNEL_ANNOUNCEMENT to l2, but not CHANNEL_UDPATE. - l3 = node_factory.get_node(disconnect=['+WIRE_CHANNEL_ANNOUNCEMENT'], - options={'dev-no-reconnect': None}, - may_reconnect=True) - l4 = node_factory.get_node(may_reconnect=True) - - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l2.rpc.connect(l3.info['id'], 'localhost', l3.port) - l3.rpc.connect(l4.info['id'], 'localhost', l4.port) + l1, l2, l3, l4 = node_factory.line_graph(4, opts=[{'log-level': 'io'}, + {'log-level': 'io'}, + {'disconnect': ['+WIRE_CHANNEL_ANNOUNCEMENT'], + 'may_reconnect': True}, + {'may_reconnect': True}], + fundchannel=False) # Make an announced-but-not-updated channel. l3.fund_channel(l4, 10**5) @@ -1094,9 +1091,11 @@ def test_gossipwith(node_factory): def test_gossip_notices_close(node_factory, bitcoind): # We want IO logging so we can replay a channel_announce to l1; # We also *really* do feed it bad gossip! - l1 = node_factory.get_node(options={'log-level': 'io'}, - allow_bad_gossip=True) - l2, l3 = node_factory.line_graph(2) + l1, l2, l3 = node_factory.get_nodes(3, opts=[{'log-level': 'io', + 'allow_bad_gossip': True}, + {}, + {}]) + node_factory.join_nodes([l2, l3]) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) bitcoind.generate_block(5) @@ -1174,7 +1173,8 @@ def test_getroute_exclude_duplicate(node_factory): @unittest.skipIf(not DEVELOPER, "gossip propagation is slow without DEVELOPER=1") def test_getroute_exclude(node_factory, bitcoind): """Test getroute's exclude argument""" - l1, l2, l3, l4 = node_factory.line_graph(4, wait_for_announce=True) + l1, l2, l3, l4, l5 = node_factory.get_nodes(5) + node_factory.join_nodes([l1, l2, l3, l4], wait_for_announce=True) # This should work route = l1.rpc.getroute(l4.info['id'], 1, 1)['route'] @@ -1230,7 +1230,6 @@ def test_getroute_exclude(node_factory, bitcoind): with pytest.raises(RpcError): l1.rpc.getroute(l4.info['id'], 1, 1, exclude=[l3.info['id'], chan_l2l4]) - l5 = node_factory.get_node() l1.rpc.connect(l5.info['id'], 'localhost', l5.port) scid15 = l1.fund_channel(l5, 1000000, wait_for_active=False) l5.rpc.connect(l4.info['id'], 'localhost', l4.port) @@ -1520,10 +1519,10 @@ def test_gossip_announce_unknown_block(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "gossip without DEVELOPER=1 is slow") def test_gossip_no_backtalk(node_factory): - l1, l2 = node_factory.line_graph(2, wait_for_announce=True) - - # This connects, gets gossip, but should *not* play it back. - l3 = node_factory.get_node(options={'log-level': 'io'}) + # l3 connects, gets gossip, but should *not* play it back. + l1, l2, l3 = node_factory.get_nodes(3, + opts=[{}, {}, {'log-level': 'io'}]) + node_factory.join_nodes([l1, l2], wait_for_announce=True) l3.rpc.connect(l2.info['id'], 'localhost', l2.port) # Will get channel_announce, then two channel_update and two node_announcement @@ -1539,12 +1538,13 @@ def test_gossip_no_backtalk(node_factory): @unittest.skipIf(not DEVELOPER, "Needs --dev-gossip") @unittest.skipIf(TEST_NETWORK != 'regtest', "Channel announcement contains genesis hash, receiving node discards on mismatch") def test_gossip_ratelimit(node_factory): + l1, l2, l3 = node_factory.get_nodes(3, + opts=[{}, {}, {'dev-gossip-time': 1568096251}]) # These make the channel exist, but we use our own gossip. - l1, l2 = node_factory.line_graph(2, wait_for_announce=True) + node_factory.join_nodes([l1, l2], wait_for_announce=True) # Here are some ones I generated earlier (by removing gossip # ratelimiting) - l3 = node_factory.get_node(options={'dev-gossip-time': 1568096251}) subprocess.run(['devtools/gossipwith', '--max-messages=0', '{}@localhost:{}'.format(l3.info['id'], l3.port), diff --git a/tests/test_invoices.py b/tests/test_invoices.py index 6068a1ea569d..ef0ac1241b20 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -183,7 +183,8 @@ def test_invoice_routeboost(node_factory, bitcoind): def test_invoice_routeboost_private(node_factory, bitcoind): """Test routeboost 'r' hint in bolt11 invoice for private channels """ - l1, l2 = node_factory.line_graph(2, fundamount=16777215, announce_channels=False) + l1, l2, l3 = node_factory.get_nodes(3) + node_factory.join_nodes([l1, l2], fundamount=16777215, announce_channels=False) scid = l1.get_channel_scid(l2) @@ -245,7 +246,6 @@ def test_invoice_routeboost_private(node_factory, bitcoind): # The existence of a public channel, even without capacity, will suppress # the exposure of private channels. - l3 = node_factory.get_node() l3.rpc.connect(l2.info['id'], 'localhost', l2.port) scid2 = l3.fund_channel(l2, (10**5)) bitcoind.generate_block(5) diff --git a/tests/test_misc.py b/tests/test_misc.py index 7e50d40bf62b..18a909981750 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -35,8 +35,7 @@ def test_stop_pending_fundchannel(node_factory, executor): freeing the daemon, but that needs a DB transaction to be open. """ - l1 = node_factory.get_node() - l2 = node_factory.get_node() + l1, l2 = node_factory.get_nodes(2) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) @@ -69,8 +68,8 @@ def test_names(node_factory): ('0265b6ab5ec860cd257865d61ef0bbf5b3339c36cbda8b26b74e7f1dca490b6518', 'LOUDPHOTO', '0265b6') ] - for key, alias, color in configs: - n = node_factory.get_node() + nodes = node_factory.get_nodes(len(configs)) + for n, (key, alias, color) in zip(nodes, configs): assert n.daemon.is_in_log(r'public key {}, alias {}.* \(color #{}\)' .format(key, alias, color)) @@ -180,17 +179,19 @@ def mock_getblock(r): } # Start it, establish channel, get extra funds. - l1, l2 = node_factory.line_graph(2, opts={'may_reconnect': True, 'wait_for_bitcoind_sync': False}) + l1, l2, l3 = node_factory.get_nodes(3, opts=[{'may_reconnect': True, + 'wait_for_bitcoind_sync': False}, + {'may_reconnect': True, + 'wait_for_bitcoind_sync': False}, + {}]) + node_factory.join_nodes([l1, l2]) # Balance l1<->l2 channel l1.pay(l2, 10**9 // 2) l1.stop() - # Start extra node. - l3 = node_factory.get_node() - - # Now make sure it's behind. + # Now make sure l2 is behind. bitcoind.generate_block(2) # Make sure l2/l3 are synced sync_blockheight(bitcoind, [l2, l3]) @@ -463,11 +464,7 @@ def test_htlc_in_timeout(node_factory, bitcoind, executor): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") def test_bech32_funding(node_factory, chainparams): # Don't get any funds from previous runs. - l1 = node_factory.get_node(random_hsm=True) - l2 = node_factory.get_node(random_hsm=True) - - # connect - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + l1, l2 = node_factory.line_graph(2, opts={'random_hsm': True}, fundchannel=False) # fund a bech32 address and then open a channel with it res = l1.openchannel(l2, 20000, 'bech32') @@ -1380,25 +1377,16 @@ def test_reserve_enforcement(node_factory, executor): def test_htlc_send_timeout(node_factory, bitcoind, compat): """Test that we don't commit an HTLC to an unreachable node.""" # Feerates identical so we don't get gratuitous commit to update them - l1 = node_factory.get_node( - options={'log-level': 'io'}, - feerates=(7500, 7500, 7500, 7500) - ) + l1, l2, l3 = node_factory.line_graph(3, opts=[{'log-level': 'io', + 'feerates': (7500, 7500, 7500, 7500)}, + # Blackhole it after it sends HTLC_ADD to l3. + {'log-level': 'io', + 'feerates': (7500, 7500, 7500, 7500), + 'disconnect': ['0WIRE_UPDATE_ADD_HTLC']}, + {}], + wait_for_announce=True) - # Blackhole it after it sends HTLC_ADD to l3. - l2 = node_factory.get_node(disconnect=['0WIRE_UPDATE_ADD_HTLC'], - options={'log-level': 'io'}, - feerates=(7500, 7500, 7500, 7500)) - l3 = node_factory.get_node() - - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - l2.rpc.connect(l3.info['id'], 'localhost', l3.port) - - l1.fund_channel(l2, 10**6) - chanid2 = l2.fund_channel(l3, 10**6) - - # Make sure channels get announced. - bitcoind.generate_block(5) + chanid2 = l2.get_channel_scid(l3) # Make sure we have 30 seconds without any incoming traffic from l3 to l2 # so it tries to ping before sending WIRE_COMMITMENT_SIGNED. @@ -2247,8 +2235,8 @@ def test_sendcustommsg(node_factory): """ plugin = os.path.join(os.path.dirname(__file__), "plugins", "custommsg.py") opts = {'log-level': 'io', 'plugin': plugin} - l1, l2, l3 = node_factory.line_graph(3, opts=opts) - l4 = node_factory.get_node(options=opts) + l1, l2, l3, l4 = node_factory.get_nodes(4, opts=opts) + node_factory.join_nodes([l1, l2, l3]) l2.connect(l4) l3.stop() msg = r'ff' * 32 diff --git a/tests/test_pay.py b/tests/test_pay.py index 8d22f86589b4..5cc8d4668a36 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -142,9 +142,12 @@ def test_pay_exclude_node(node_factory, bitcoind): opts = [ {'disable-mpp': None}, {'plugin': os.path.join(os.getcwd(), 'tests/plugins/fail_htlcs.py')}, + {}, + {'fee-base': 100, 'fee-per-satoshi': 1000}, {} ] - l1, l2, l3 = node_factory.line_graph(3, opts=opts, wait_for_announce=True) + l1, l2, l3, l4, l5 = node_factory.get_nodes(5, opts=opts) + node_factory.join_nodes([l1, l2, l3], wait_for_announce=True) amount = 10**8 inv = l3.rpc.invoice(amount, "test1", 'description')['bolt11'] @@ -169,8 +172,6 @@ def test_pay_exclude_node(node_factory, bitcoind): # l1->l4->l5->l3 is the longer route. This makes sure this route won't be # tried for the first pay attempt. Just to be sure we also raise the fees # that l4 leverages. - l4 = node_factory.get_node(options={'fee-base': 100, 'fee-per-satoshi': 1000}) - l5 = node_factory.get_node() l1.rpc.connect(l4.info['id'], 'localhost', l4.port) l4.rpc.connect(l5.info['id'], 'localhost', l5.port) l5.rpc.connect(l3.info['id'], 'localhost', l3.port) @@ -1072,9 +1073,9 @@ def test_forward_different_fees_and_cltv(node_factory, bitcoind): # 1. D: 400 base + 4000 millionths # We don't do D yet. - l1 = node_factory.get_node(options={'cltv-delta': 10, 'fee-base': 100, 'fee-per-satoshi': 1000}) - l2 = node_factory.get_node(options={'cltv-delta': 20, 'fee-base': 200, 'fee-per-satoshi': 2000}) - l3 = node_factory.get_node(options={'cltv-delta': 30, 'cltv-final': 9, 'fee-base': 300, 'fee-per-satoshi': 3000}) + l1, l2, l3 = node_factory.get_nodes(3, opts=[{'cltv-delta': 10, 'fee-base': 100, 'fee-per-satoshi': 1000}, + {'cltv-delta': 20, 'fee-base': 200, 'fee-per-satoshi': 2000}, + {'cltv-delta': 30, 'cltv-final': 9, 'fee-base': 300, 'fee-per-satoshi': 3000}]) ret = l1.rpc.connect(l2.info['id'], 'localhost', l2.port) assert ret['id'] == l2.info['id'] @@ -1178,9 +1179,9 @@ def test_forward_different_fees_and_cltv(node_factory, bitcoind): def test_forward_pad_fees_and_cltv(node_factory, bitcoind): """Test that we are allowed extra locktime delta, and fees""" - l1 = node_factory.get_node(options={'cltv-delta': 10, 'fee-base': 100, 'fee-per-satoshi': 1000}) - l2 = node_factory.get_node(options={'cltv-delta': 20, 'fee-base': 200, 'fee-per-satoshi': 2000}) - l3 = node_factory.get_node(options={'cltv-delta': 30, 'cltv-final': 9, 'fee-base': 300, 'fee-per-satoshi': 3000}) + l1, l2, l3 = node_factory.get_nodes(3, opts=[{'cltv-delta': 10, 'fee-base': 100, 'fee-per-satoshi': 1000}, + {'cltv-delta': 20, 'fee-base': 200, 'fee-per-satoshi': 2000}, + {'cltv-delta': 30, 'cltv-final': 9, 'fee-base': 300, 'fee-per-satoshi': 3000}]) ret = l1.rpc.connect(l2.info['id'], 'localhost', l2.port) assert ret['id'] == l2.info['id'] @@ -1236,9 +1237,8 @@ def test_forward_stats(node_factory, bitcoind): """ amount = 10**5 - l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=False) - l4 = node_factory.get_node() - l5 = node_factory.get_node(may_fail=True) + l1, l2, l3, l4, l5 = node_factory.get_nodes(5, opts=[{}] * 4 + [{'may_fail': True}]) + node_factory.join_nodes([l1, l2, l3], wait_for_announce=False) l2.openchannel(l4, 10**6, wait_for_announce=False) l2.openchannel(l5, 10**6, wait_for_announce=True) @@ -1347,12 +1347,12 @@ def test_forward_local_failed_stats(node_factory, bitcoind, executor): disconnects = ['-WIRE_UPDATE_FAIL_HTLC', 'permfail'] - l1 = node_factory.get_node() - l2 = node_factory.get_node() - l3 = node_factory.get_node() - l4 = node_factory.get_node(disconnect=disconnects) - l5 = node_factory.get_node() - l6 = node_factory.get_node() + l1, l2, l3, l4, l5, l6 = node_factory.get_nodes(6, opts=[{}, + {}, + {}, + {'disconnect': disconnects}, + {}, + {}]) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l2.rpc.connect(l3.info['id'], 'localhost', l3.port) @@ -1813,12 +1813,10 @@ def test_setchannelfee_usage(node_factory, bitcoind): DEF_BASE = 10 DEF_PPM = 100 - l1 = node_factory.get_node(options={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) - l2 = node_factory.get_node(options={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) - l3 = node_factory.get_node(options={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + l1, l2, l3 = node_factory.get_nodes(3, + opts={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) + node_factory.join_nodes([l1, l2]) l1.rpc.connect(l3.info['id'], 'localhost', l3.port) - l1.fund_channel(l2, 1000000) def channel_get_fees(scid): return l1.db.query( @@ -1933,9 +1931,7 @@ def test_setchannelfee_state(node_factory, bitcoind): DEF_BASE = 0 DEF_PPM = 0 - l0 = node_factory.get_node(options={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) - l1 = node_factory.get_node(options={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) - l2 = node_factory.get_node(options={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) + l0, l1, l2 = node_factory.get_nodes(3, opts={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) # connection and funding l0.rpc.connect(l1.info['id'], 'localhost', l1.port) @@ -2132,9 +2128,7 @@ def test_setchannelfee_all(node_factory, bitcoind): DEF_BASE = 10 DEF_PPM = 100 - l1 = node_factory.get_node(options={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) - l2 = node_factory.get_node(options={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) - l3 = node_factory.get_node(options={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) + l1, l2, l3 = node_factory.get_nodes(3, opts={'fee-base': DEF_BASE, 'fee-per-satoshi': DEF_PPM}) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.rpc.connect(l3.info['id'], 'localhost', l3.port) l1.fund_channel(l2, 1000000) @@ -2351,16 +2345,11 @@ def test_error_returns_blockheight(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, 'Needs dev-routes') def test_tlv_or_legacy(node_factory, bitcoind): - l1, l2, l3 = node_factory.get_nodes(3, - opts={'plugin': os.path.join(os.getcwd(), 'tests/plugins/print_htlc_onion.py')}) - - # Set up a channel from 1->2 first. - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - scid12 = l1.fund_channel(l2, 1000000) + l1, l2, l3 = node_factory.line_graph(3, + opts={'plugin': os.path.join(os.getcwd(), 'tests/plugins/print_htlc_onion.py')}) - # Now set up 2->3. - l2.rpc.connect(l3.info['id'], 'localhost', l3.port) - scid23 = l2.fund_channel(l3, 1000000) + scid12 = l1.get_channel_scid(l2) + scid23 = l2.get_channel_scid(l3) # We need to force l3 to provide route hint from l2 (it won't normally, # since it sees l2 as a dead end). @@ -2378,8 +2367,8 @@ def test_tlv_or_legacy(node_factory, bitcoind): l2.daemon.wait_for_log("Got onion.*'type': 'legacy'") l3.daemon.wait_for_log("Got onion.*'type': 'tlv'") - # Turns out we only need 3 more blocks to announce l1->l2 channel. - bitcoind.generate_block(3) + # We need 5 more blocks to announce l1->l2 channel. + bitcoind.generate_block(5) # Make sure l1 knows about l2 wait_for(lambda: 'alias' in l1.rpc.listnodes(l2.info['id'])['nodes'][0]) @@ -3031,7 +3020,7 @@ def test_pay_exemptfee(node_factory, compat): @unittest.skipIf(not DEVELOPER, "Requires use_shadow flag") -def test_pay_peer(node_factory): +def test_pay_peer(node_factory, bitcoind): """If we have a direct channel to the destination we should use that. This is complicated a bit by not having sufficient capacity, but the @@ -3042,11 +3031,10 @@ def test_pay_peer(node_factory): v / l3 """ - l1, l2 = node_factory.line_graph(2, fundamount=10**6) - l3 = node_factory.get_node() - - l1.openchannel(l3, 10**6, wait_for_announce=False) - l3.openchannel(l2, 10**6, wait_for_announce=True) + l1, l2, l3 = node_factory.get_nodes(3) + node_factory.join_nodes([l1, l2]) + node_factory.join_nodes([l1, l3]) + node_factory.join_nodes([l3, l2], wait_for_announce=True) wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 6) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index ae6eecee24f4..5415f66715ac 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -789,13 +789,14 @@ def test_forward_event_notification(node_factory, bitcoind, executor): amount = 10**8 disconnects = ['-WIRE_UPDATE_FAIL_HTLC', 'permfail'] - l1, l2, l3 = node_factory.line_graph(3, opts=[ + l1, l2, l3, l4, l5 = node_factory.get_nodes(5, opts=[ {}, {'plugin': os.path.join(os.getcwd(), 'tests/plugins/forward_payment_status.py')}, - {} - ], wait_for_announce=True) - l4 = node_factory.get_node() - l5 = node_factory.get_node(disconnect=disconnects) + {}, + {}, + {'disconnect': disconnects}]) + + node_factory.join_nodes([l1, l2, l3], wait_for_announce=True) l2.openchannel(l4, 10**6, wait_for_announce=False) l2.openchannel(l5, 10**6, wait_for_announce=True) From c2a40eff35ce4b891fe46b4af5b3ada3f16b52c5 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 7 Aug 2020 12:44:58 +0930 Subject: [PATCH 08/10] pytest: optimize join_nodes a little. We can query all the txids at once, rather than one at a time. Doesn't make any measurable difference to full runtime testing here though. Signed-off-by: Rusty Russell --- contrib/pyln-testing/pyln/testing/utils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index a9616bea1b89..e8b3bfda537a 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -1095,9 +1095,11 @@ def join_nodes(self, nodes, fundchannel=True, fundamount=10**6, wait_for_announc bitcoind.generate_block(1) sync_blockheight(bitcoind, nodes) + txids = [] for src, dst in connections: - tx = src.rpc.fundchannel(dst.info['id'], fundamount, announce=announce_channels) - wait_for(lambda: tx['txid'] in bitcoind.rpc.getrawmempool()) + txids.append(src.rpc.fundchannel(dst.info['id'], fundamount, announce=announce_channels)['txid']) + + wait_for(lambda: set(txids).issubset(set(bitcoind.rpc.getrawmempool()))) # Confirm all channels and wait for them to become usable bitcoind.generate_block(1) From a8259b2fe759426b03967a6edcdeb4cef949cef2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 7 Aug 2020 12:44:59 +0930 Subject: [PATCH 09/10] pytest: make valgrind a per-node option. Next patch will turn it off for slow-marked tests. Signed-off-by: Rusty Russell --- contrib/pyln-testing/pyln/testing/utils.py | 14 ++++++++------ tests/test_misc.py | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index e8b3bfda537a..d0a6bcaf751c 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -550,9 +550,10 @@ def wait(self, timeout=10): class LightningNode(object): - def __init__(self, node_id, lightning_dir, bitcoind, executor, may_fail=False, + def __init__(self, node_id, lightning_dir, bitcoind, executor, valgrind, may_fail=False, may_reconnect=False, allow_broken_log=False, - allow_bad_gossip=False, db=None, port=None, disconnect=None, random_hsm=None, options=None, **kwargs): + allow_bad_gossip=False, db=None, port=None, disconnect=None, random_hsm=None, options=None, + **kwargs): self.bitcoin = bitcoind self.executor = executor self.may_fail = may_fail @@ -585,7 +586,7 @@ def __init__(self, node_id, lightning_dir, bitcoind, executor, may_fail=False, self.daemon.env["LIGHTNINGD_DEV_MEMLEAK"] = "1" if os.getenv("DEBUG_SUBD"): self.daemon.opts["dev-debugger"] = os.getenv("DEBUG_SUBD") - if VALGRIND: + if valgrind: self.daemon.env["LIGHTNINGD_DEV_NO_BACKTRACE"] = "1" if not may_reconnect: self.daemon.opts["dev-no-reconnect"] = None @@ -595,7 +596,7 @@ def __init__(self, node_id, lightning_dir, bitcoind, executor, may_fail=False, dsn = db.get_dsn() if dsn is not None: self.daemon.opts['wallet'] = dsn - if VALGRIND: + if valgrind: self.daemon.cmd_prefix = [ 'valgrind', '-q', @@ -968,6 +969,7 @@ def __init__(self, testname, bitcoind, executor, directory, db_provider, node_cl self.lock = threading.Lock() self.db_provider = db_provider self.node_cls = node_cls + self.valgrind = VALGRIND def split_options(self, opts): """Split node options from cli options @@ -1042,7 +1044,7 @@ def get_node(self, node_id=None, options=None, dbfile=None, # node. db = self.db_provider.get_db(os.path.join(lightning_dir, TEST_NETWORK), self.testname, node_id) node = self.node_cls( - node_id, lightning_dir, self.bitcoind, self.executor, db=db, + node_id, lightning_dir, self.bitcoind, self.executor, self.valgrind, db=db, port=port, options=options, may_fail=may_fail or expect_fail, **kwargs ) @@ -1165,7 +1167,7 @@ def killall(self, expected_successes): leaks = None # leak detection upsets VALGRIND by reading uninitialized mem. # If it's dead, we'll catch it below. - if not VALGRIND and DEVELOPER: + if not self.valgrind and DEVELOPER: try: # This also puts leaks in log. leaks = self.nodes[i].rpc.dev_memleak()['leaks'] diff --git a/tests/test_misc.py b/tests/test_misc.py index 18a909981750..e09fcb680900 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -2174,7 +2174,7 @@ def test_unix_socket_path_length(node_factory, bitcoind, directory, executor, db os.makedirs(lightning_dir) db = db_provider.get_db(lightning_dir, "test_unix_socket_path_length", 1) - l1 = LightningNode(1, lightning_dir, bitcoind, executor, db=db, port=node_factory.get_next_port()) + l1 = LightningNode(1, lightning_dir, bitcoind, executor, VALGRIND, db=db, port=node_factory.get_next_port()) # `LightningNode.start()` internally calls `LightningRpc.getinfo()` which # exercises the socket logic, and raises an issue if it fails. From 6c9d196807e5661c54029e2afa6b91037fd23fe3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 7 Aug 2020 12:44:59 +0930 Subject: [PATCH 10/10] pytest: add slow_test marker. And when it's set, and we're SLOW_MACHINE, simply disable valgrind. Since Travis (SLOW_MACHINE=1) only does VALGRIND=1 DEVELOPER=1 tests, and VALGRIND=0 DEVELOPER=0 tests, it was missing tests which needed DEVELOPER and !VALGRIND. Instead, this demotes them to non-valgrind tests for SLOW_MACHINEs. Signed-off-by: Rusty Russell --- contrib/pyln-testing/pyln/testing/fixtures.py | 1 + contrib/pyln-testing/pyln/testing/utils.py | 7 ++++-- tests/test_closing.py | 22 +++++++++---------- tests/test_connection.py | 15 ++++++++----- tests/test_pay.py | 14 +++++++----- tests/utils.py | 2 +- 6 files changed, 34 insertions(+), 27 deletions(-) diff --git a/contrib/pyln-testing/pyln/testing/fixtures.py b/contrib/pyln-testing/pyln/testing/fixtures.py index 3f61bc9301d3..3f32a6561383 100644 --- a/contrib/pyln-testing/pyln/testing/fixtures.py +++ b/contrib/pyln-testing/pyln/testing/fixtures.py @@ -174,6 +174,7 @@ def teardown_checks(request): @pytest.fixture def node_factory(request, directory, test_name, bitcoind, executor, db_provider, teardown_checks, node_cls): nf = NodeFactory( + request, test_name, bitcoind, executor, diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index d0a6bcaf751c..37b5900a62e5 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -959,7 +959,11 @@ def passes_filters(hmsg, filters): class NodeFactory(object): """A factory to setup and start `lightningd` daemons. """ - def __init__(self, testname, bitcoind, executor, directory, db_provider, node_cls): + def __init__(self, request, testname, bitcoind, executor, directory, db_provider, node_cls): + if request.node.get_closest_marker("slow_test") and SLOW_MACHINE: + self.valgrind = False + else: + self.valgrind = VALGRIND self.testname = testname self.next_id = 1 self.nodes = [] @@ -969,7 +973,6 @@ def __init__(self, testname, bitcoind, executor, directory, db_provider, node_cl self.lock = threading.Lock() self.db_provider = db_provider self.node_cls = node_cls - self.valgrind = VALGRIND def split_options(self, opts): """Split node options from cli options diff --git a/tests/test_closing.py b/tests/test_closing.py index 193f2471fc04..503480fb53bc 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -3,8 +3,8 @@ from pyln.client import RpcError from shutil import copyfile from utils import ( - only_one, sync_blockheight, wait_for, DEVELOPER, TIMEOUT, VALGRIND, - SLOW_MACHINE, account_balance, first_channel_id + only_one, sync_blockheight, wait_for, DEVELOPER, TIMEOUT, + account_balance, first_channel_id ) import os @@ -149,17 +149,15 @@ def test_closing_id(node_factory): wait_for(lambda: not only_one(l2.rpc.listpeers(l1.info['id'])['peers'])['connected']) -@unittest.skipIf(VALGRIND, "Flaky under valgrind") +@pytest.mark.slow_test def test_closing_torture(node_factory, executor, bitcoind): # We set up a fully-connected mesh of N nodes, then try # closing them all at once. amount = 10**6 num_nodes = 10 # => 45 channels (36 seconds on my laptop) - if VALGRIND: + if node_factory.valgrind: num_nodes -= 4 # => 15 (135 seconds) - if SLOW_MACHINE: - num_nodes -= 1 # => 36/10 (37/95 seconds) nodes = node_factory.get_nodes(num_nodes) @@ -213,7 +211,7 @@ def test_closing_torture(node_factory, executor, bitcoind): wait_for(lambda: n.rpc.listpeers()['peers'] == []) -@unittest.skipIf(SLOW_MACHINE and VALGRIND, "slow test") +@pytest.mark.slow_test def test_closing_different_fees(node_factory, bitcoind, executor): l1 = node_factory.get_node() @@ -690,8 +688,8 @@ def test_penalty_outhtlc(node_factory, bitcoind, executor, chainparams): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") -@unittest.skipIf(SLOW_MACHINE and VALGRIND, "slow test") @unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "Makes use of the sqlite3 db") +@pytest.mark.slow_test def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams): """ Test that the penalizing node claims any published HTLC transactions @@ -823,8 +821,8 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") -@unittest.skipIf(SLOW_MACHINE and VALGRIND, "slow test") @unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "Makes use of the sqlite3 db") +@pytest.mark.slow_test def test_penalty_htlc_tx_timeout(node_factory, bitcoind, chainparams): """ Test that the penalizing node claims any published HTLC transactions @@ -1866,7 +1864,7 @@ def setup_multihtlc_test(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1 for dev_ignore_htlcs") -@unittest.skipIf(SLOW_MACHINE and VALGRIND, "slow test") +@pytest.mark.slow_test def test_onchain_multihtlc_our_unilateral(node_factory, bitcoind): """Node pushes a channel onchain with multiple HTLCs with same payment_hash """ h, nodes = setup_multihtlc_test(node_factory, bitcoind) @@ -1958,7 +1956,7 @@ def test_onchain_multihtlc_our_unilateral(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1 for dev_ignore_htlcs") -@unittest.skipIf(SLOW_MACHINE and VALGRIND, "slow test") +@pytest.mark.slow_test def test_onchain_multihtlc_their_unilateral(node_factory, bitcoind): """Node pushes a channel onchain with multiple HTLCs with same payment_hash """ h, nodes = setup_multihtlc_test(node_factory, bitcoind) @@ -2257,7 +2255,7 @@ def check_billboard(): def test_shutdown(node_factory): # Fail, in that it will exit before cleanup. l1 = node_factory.get_node(may_fail=True) - if not VALGRIND: + if not node_factory.valgrind: leaks = l1.rpc.dev_memleak()['leaks'] if len(leaks): raise Exception("Node {} has memory leaks: {}" diff --git a/tests/test_connection.py b/tests/test_connection.py index d099706fb8f6..69957b9f6e8f 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -5,8 +5,8 @@ from flaky import flaky # noqa: F401 from pyln.client import RpcError, Millisatoshi from utils import ( - DEVELOPER, only_one, wait_for, sync_blockheight, VALGRIND, TIMEOUT, - SLOW_MACHINE, expected_peer_features, expected_node_features, + DEVELOPER, only_one, wait_for, sync_blockheight, TIMEOUT, + expected_peer_features, expected_node_features, expected_channel_features, check_coin_moves, first_channel_id, account_balance ) @@ -140,6 +140,7 @@ def test_bad_opening(node_factory): @unittest.skipIf(not DEVELOPER, "gossip without DEVELOPER=1 is slow") @unittest.skipIf(TEST_NETWORK != 'regtest', "Fee computation and limits are network specific") +@pytest.mark.slow_test def test_opening_tiny_channel(node_factory): # Test custom min-capacity-sat parameters # @@ -990,7 +991,7 @@ def test_funding_external_wallet_corners(node_factory, bitcoind): def test_funding_cancel_race(node_factory, bitcoind, executor): l1 = node_factory.get_node() - if VALGRIND or SLOW_MACHINE: + if node_factory.valgrind: num = 5 else: num = 100 @@ -1051,7 +1052,7 @@ def test_funding_cancel_race(node_factory, bitcoind, executor): assert num_cancel == len(nodes) # We should have raced at least once! - if not VALGRIND: + if not node_factory.valgrind: assert num_cancel > 0 assert num_complete > 0 @@ -1994,11 +1995,12 @@ def test_fulfill_incoming_first(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "gossip without DEVELOPER=1 is slow") +@pytest.mark.slow_test def test_restart_many_payments(node_factory, bitcoind): l1 = node_factory.get_node(may_reconnect=True) # On my laptop, these take 89 seconds and 12 seconds - if VALGRIND: + if node_factory.valgrind: num = 2 else: num = 5 @@ -2235,9 +2237,10 @@ def test_feerate_stress(node_factory, executor): @unittest.skipIf(not DEVELOPER, "need dev_disconnect") +@pytest.mark.slow_test def test_pay_disconnect_stress(node_factory, executor): """Expose race in htlc restoration in channeld: 50% chance of failure""" - if SLOW_MACHINE and VALGRIND: + if node_factory.valgrind: NUM_RUNS = 2 else: NUM_RUNS = 5 diff --git a/tests/test_pay.py b/tests/test_pay.py index 5cc8d4668a36..5e59a132d31f 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -5,8 +5,8 @@ from pyln.client import RpcError, Millisatoshi from pyln.proto.onion import TlvPayload from utils import ( - DEVELOPER, wait_for, only_one, sync_blockheight, SLOW_MACHINE, TIMEOUT, - VALGRIND, EXPERIMENTAL_FEATURES + DEVELOPER, wait_for, only_one, sync_blockheight, TIMEOUT, + EXPERIMENTAL_FEATURES ) import copy import os @@ -1303,7 +1303,8 @@ def test_forward_stats(node_factory, bitcoind): assert 'received_time' in stats['forwards'][2] and 'resolved_time' not in stats['forwards'][2] -@unittest.skipIf(not DEVELOPER or (VALGRIND and SLOW_MACHINE), "Gossip too slow without DEVELOPER, and too stressful if VALGRIND on slow machines") +@unittest.skipIf(not DEVELOPER, "too slow without --dev-fast-gossip") +@pytest.mark.slow_test def test_forward_local_failed_stats(node_factory, bitcoind, executor): """Check that we track forwarded payments correctly. @@ -1522,7 +1523,8 @@ def test_forward_local_failed_stats(node_factory, bitcoind, executor): assert 'received_time' in stats['forwards'][3] and 'resolved_time' not in stats['forwards'][4] -@unittest.skipIf(not DEVELOPER or SLOW_MACHINE, "needs DEVELOPER=1 for dev_ignore_htlcs, and temporarily disabled on Travis") +@unittest.skipIf(not DEVELOPER, "too slow without --dev-fast-gossip") +@pytest.mark.slow_test def test_htlcs_cltv_only_difference(node_factory, bitcoind): # l1 -> l2 -> l3 -> l4 # l4 ignores htlcs, so they stay. @@ -1599,7 +1601,7 @@ def test_pay_variants(node_factory): @unittest.skipIf(not DEVELOPER, "gossip without DEVELOPER=1 is slow") -@unittest.skipIf(VALGRIND and SLOW_MACHINE, "Travis times out under valgrind") +@pytest.mark.slow_test def test_pay_retry(node_factory, bitcoind, executor, chainparams): """Make sure pay command retries properly. """ @@ -1683,7 +1685,7 @@ def listpays_nofail(b11): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1 otherwise gossip takes 5 minutes!") -@unittest.skipIf(VALGRIND, "temporarily disabled due to timeouts") +@pytest.mark.slow_test def test_pay_routeboost(node_factory, bitcoind, compat): """Make sure we can use routeboost information. """ # l1->l2->l3--private-->l4 diff --git a/tests/utils.py b/tests/utils.py index 84366439369b..f7a337e16453 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,4 +1,4 @@ -from pyln.testing.utils import TEST_NETWORK, SLOW_MACHINE, TIMEOUT, VALGRIND, DEVELOPER, DEPRECATED_APIS # noqa: F401 +from pyln.testing.utils import TEST_NETWORK, TIMEOUT, VALGRIND, DEVELOPER, DEPRECATED_APIS # noqa: F401 from pyln.testing.utils import env, only_one, wait_for, write_config, TailableProc, sync_blockheight, wait_channel_quiescent, get_tx_p2wsh_outnum # noqa: F401 import bitstring from pyln.client import Millisatoshi