diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index 6f52f213b310..8978a876641a 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -957,9 +957,6 @@ def fundbalancedchannel(self, remote_node, total_capacity=FUNDAMOUNT, announce=T return '{}x{}x{}'.format(self.bitcoin.rpc.getblockcount(), txnum, res['outnum']) - def getactivechannels(self): - return [c for c in self.rpc.listchannels()['channels'] if c['active']] - def db_query(self, query): return self.db.query(query) @@ -1065,8 +1062,8 @@ def has_funds_on_addr(addr): txnum, res['outnum']) if wait_for_active: - self.wait_channel_active(scid) - l2.wait_channel_active(scid) + self.wait_local_channel_active(scid) + l2.wait_local_channel_active(scid) return scid, res @@ -1114,7 +1111,16 @@ def get_channel_id(self, other): return None return channels[0]['channel_id'] + def is_local_channel_active(self, scid): + """Is the local channel @scid usable?""" + channels = self.rpc.listpeerchannels()['channels'] + return [c['state'] in ('CHANNELD_NORMAL', 'CHANNELD_AWAITING_SPLICE') for c in channels if c.get('short_channel_id') == scid] == [True] + + def wait_local_channel_active(self, scid): + wait_for(lambda: self.is_local_channel_active(scid)) + def is_channel_active(self, chanid): + """Does gossip show this channel as enabled both ways?""" channels = self.rpc.listchannels(chanid)['channels'] active = [(c['short_channel_id'], c['channel_flags']) for c in channels if c['active']] return (chanid, 0) in active and (chanid, 1) in active @@ -1123,8 +1129,8 @@ def wait_for_channel_onchain(self, peerid): txid = only_one(self.rpc.listpeerchannels(peerid)['channels'])['scratch_txid'] wait_for(lambda: txid in self.bitcoin.rpc.getrawmempool()) - def wait_channel_active(self, chanid): - wait_for(lambda: self.is_channel_active(chanid)) + def wait_channel_active(self, scid): + wait_for(lambda: self.is_channel_active(scid)) # This waits until gossipd sees channel_update in both directions # (or for local channels, at least a local announcement) @@ -1606,8 +1612,8 @@ def join_nodes(self, nodes, fundchannel=True, fundamount=FUNDAMOUNT, wait_for_an # Wait for all channels to be active (locally) for i, n in enumerate(scids): - nodes[i].wait_channel_active(scids[i]) - nodes[i + 1].wait_channel_active(scids[i]) + nodes[i].wait_local_channel_active(scids[i]) + nodes[i + 1].wait_local_channel_active(scids[i]) if not wait_for_announce: return diff --git a/tests/test_bookkeeper.py b/tests/test_bookkeeper.py index a65a46509b2c..9788d484c555 100644 --- a/tests/test_bookkeeper.py +++ b/tests/test_bookkeeper.py @@ -367,7 +367,7 @@ def test_bookkeeping_missed_chans_leases(node_factory, bitcoind): bitcoind.generate_block(1, wait_for_mempool=[txid]) wait_for(lambda: l1.channel_state(l2) == 'CHANNELD_NORMAL') scid = l1.get_channel_scid(l2) - l1.wait_channel_active(scid) + l1.wait_local_channel_active(scid) channel_id = first_channel_id(l1, l2) l1.pay(l2, invoice_msat) @@ -432,7 +432,7 @@ def test_bookkeeping_missed_chans_pushed(node_factory, bitcoind): bitcoind.generate_block(1, wait_for_mempool=[txid]) wait_for(lambda: l1.channel_state(l2) == 'CHANNELD_NORMAL') scid = l1.get_channel_scid(l2) - l1.wait_channel_active(scid) + l1.wait_local_channel_active(scid) channel_id = first_channel_id(l1, l2) # Send l2 funds via the channel @@ -499,7 +499,7 @@ def test_bookkeeping_missed_chans_pay_after(node_factory, bitcoind): bitcoind.generate_block(1, wait_for_mempool=[txid]) wait_for(lambda: l1.channel_state(l2) == 'CHANNELD_NORMAL') scid = l1.get_channel_scid(l2) - l1.wait_channel_active(scid) + l1.wait_local_channel_active(scid) channel_id = first_channel_id(l1, l2) # Now turn the bookkeeper on and restart @@ -520,7 +520,8 @@ def test_bookkeeping_missed_chans_pay_after(node_factory, bitcoind): assert channel_id in accts # Send a payment, should be ok. - l1.wait_channel_active(scid) + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + l1.wait_local_channel_active(scid) l1.pay(l2, invoice_msat) l1.daemon.wait_for_log(r'coin movement:.*\'invoice\'') diff --git a/tests/test_cln_rs.py b/tests/test_cln_rs.py index 96cd413cdc98..52a36f63129b 100644 --- a/tests/test_cln_rs.py +++ b/tests/test_cln_rs.py @@ -3,6 +3,7 @@ from pathlib import Path from pyln import grpc as clnpb from pyln.testing.utils import env, TEST_NETWORK, wait_for, sync_blockheight, TIMEOUT +from utils import first_scid import grpc import pytest import subprocess @@ -306,6 +307,9 @@ def test_grpc_keysend_routehint(bitcoind, node_factory): ]) ]) + # FIXME: keysend needs (unannounced) channel in gossip_store + l1.wait_channel_active(first_scid(l1, l2)) + # And now we send a keysend with that routehint list call = clnpb.KeysendRequest( destination=bytes.fromhex(l3.info['id']), diff --git a/tests/test_closing.py b/tests/test_closing.py index b7a46becff00..46f23f580395 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -39,9 +39,9 @@ def test_closing_simple(node_factory, bitcoind, chainparams): assert billboard == ['CHANNELD_NORMAL:Channel ready for use.'] bitcoind.generate_block(5) + l1.wait_channel_active(chan) + l2.wait_channel_active(chan) - wait_for(lambda: len(l1.getactivechannels()) == 2) - wait_for(lambda: len(l2.getactivechannels()) == 2) billboard = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status'] # This may either be from a local_update or an announce, so just # check for the substring @@ -61,9 +61,9 @@ def test_closing_simple(node_factory, bitcoind, chainparams): l1.daemon.wait_for_log('sendrawtx exit 0') l2.daemon.wait_for_log('sendrawtx exit 0') - # Both nodes should have disabled the channel in their view - wait_for(lambda: len(l1.getactivechannels()) == 0) - wait_for(lambda: len(l2.getactivechannels()) == 0) + # Both nodes should have disabled the channel in gossip + wait_for(lambda: not any([c['active'] for c in l1.rpc.listchannels()['channels']])) + wait_for(lambda: not any([c['active'] for c in l2.rpc.listchannels()['channels']])) assert bitcoind.rpc.getmempoolinfo()['size'] == 1 @@ -307,8 +307,9 @@ def test_closing_specified_destination(node_factory, bitcoind, chainparams): l1.daemon.wait_for_logs([' to CLOSINGD_SIGEXCHANGE'] * 3) - # Both nodes should have disabled the channel in their view - wait_for(lambda: len(l1.getactivechannels()) == 0) + # Both nodes should have disabled the channel in gossip + wait_for(lambda: not any([c['active'] for c in l1.rpc.listchannels()['channels']])) + wait_for(lambda: not any([c['active'] for c in l2.rpc.listchannels()['channels']])) wait_for(lambda: bitcoind.rpc.getmempoolinfo()['size'] == 3) @@ -508,12 +509,13 @@ def test_penalty_inhtlc(node_factory, bitcoind, executor, chainparams, anchors): opts]) channel_id = first_channel_id(l1, l2) + scid = first_scid(l1, l2) # Now, this will get stuck due to l1 commit being disabled.. t = executor.submit(l1.pay, l2, 100000000) - assert len(l1.getactivechannels()) == 2 - assert len(l2.getactivechannels()) == 2 + assert l1.is_local_channel_active(scid) + assert l2.is_local_channel_active(scid) # They should both have commitments blocked now. l1.daemon.wait_for_log('dev-disable-commit-after: disabling') @@ -549,7 +551,7 @@ def test_penalty_inhtlc(node_factory, bitcoind, executor, chainparams, anchors): l2.daemon.wait_for_log(' to ONCHAIN') # FIXME: l1 should try to stumble along! - wait_for(lambda: len(l2.getactivechannels()) == 0) + wait_for(lambda: l2.is_local_channel_active(scid) is False) # l2 should spend all of the outputs (except to-us). # Could happen in any order, depending on commitment tx. @@ -1606,13 +1608,14 @@ def test_penalty_rbf_normal(node_factory, bitcoind, executor, chainparams, ancho l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.fundchannel(l2, 10**7) channel_id = first_channel_id(l1, l2) + scid = first_scid(l1, l2) # Trigger an HTLC being added. t = executor.submit(l1.pay, l2, 1000000 * 1000) # Make sure the channel is still alive. - assert len(l1.getactivechannels()) == 2 - assert len(l2.getactivechannels()) == 2 + assert l1.is_local_channel_active(scid) + assert l2.is_local_channel_active(scid) # Wait for the disconnection. l1.daemon.wait_for_log('dev-disable-commit-after: disabling') diff --git a/tests/test_connection.py b/tests/test_connection.py index f847a6e93835..f0965d0ddd57 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -2459,11 +2459,6 @@ def test_update_fee(node_factory, bitcoind): # Make l1 send out feechange. l1.set_feerates((14000, 11000, 7500, 3750)) - # Now make sure an HTLC works. - # (First wait for route propagation.) - l1.wait_channel_active(chanid) - sync_blockheight(bitcoind, [l1, l2]) - # Make payments. l1.pay(l2, 200000000) # First payment causes fee update. diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 8f53e29ad613..995901b2c276 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -456,9 +456,8 @@ def test_gossip_jsonrpc(node_factory): needle = "Received node_announcement for node" l1.daemon.wait_for_log(needle) l2.daemon.wait_for_log(needle) - # Need to increase timeout, intervals cannot be shortened with DEVELOPER=0 - wait_for(lambda: len(l1.getactivechannels()) == 2, timeout=60) - wait_for(lambda: len(l2.getactivechannels()) == 2, timeout=60) + l1.wait_channel_active(only_one(channels1)['short_channel_id']) + l2.wait_channel_active(only_one(channels1)['short_channel_id']) nodes = l1.rpc.listnodes()['nodes'] assert set([n['nodeid'] for n in nodes]) == set([l1.info['id'], l2.info['id']]) @@ -1644,6 +1643,9 @@ def setup_gossip_store_test(node_factory, bitcoind): # Create another channel, which will stay private. scid12, _ = l1.fundchannel(l2, 10**6) + # FIXME: We assume that private announcements are in gossip_store! + l2.wait_channel_active(scid12) + # Now insert channel_update for previous channel; now they're both past the # node announcements. l3.rpc.setchannel(l2.info['id'], feebase=20, feeppm=1000) @@ -2069,8 +2071,8 @@ def test_tor_port_onions(node_factory): @pytest.mark.developer("devtools are for devs anyway") def test_routetool(node_factory): - """Test that route tool can see unpublished channels""" - l1, l2 = node_factory.line_graph(2) + """Test that route tool can see published channels""" + l1, l2 = node_factory.line_graph(2, wait_for_announce=True) subprocess.run(['devtools/route', os.path.join(l1.daemon.lightning_dir, diff --git a/tests/test_misc.py b/tests/test_misc.py index ee59d5b7fa04..0a35eaa2d0b7 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -325,9 +325,6 @@ def test_htlc_out_timeout(node_factory, bitcoind, executor): l1.rpc.connect(l2.info['id'], 'localhost', l2.port) chanid, _ = l1.fundchannel(l2, 10**6) - # Wait for route propagation. - l1.wait_channel_active(chanid) - amt = 200000000 inv = l2.rpc.invoice(amt, 'test_htlc_out_timeout', 'desc')['bolt11'] assert only_one(l2.rpc.listinvoices('test_htlc_out_timeout')['invoices'])['status'] == 'unpaid' @@ -398,7 +395,6 @@ def test_htlc_in_timeout(node_factory, bitcoind, executor): l1.rpc.connect(l2.info['id'], 'localhost', l2.port) chanid, _ = l1.fundchannel(l2, 10**6) - l1.wait_channel_active(chanid) sync_blockheight(bitcoind, [l1, l2]) amt = 200000000 @@ -1247,12 +1243,6 @@ def test_blockchaintrack(node_factory, bitcoind): assert [o for o in l1.rpc.listfunds()['outputs'] if o['status'] != "unconfirmed"] == [] -def chan_active(node, scid, is_active): - chans = node.rpc.listchannels(scid)['channels'] - print(chans) - return [c['active'] for c in chans] == [is_active, is_active] - - @pytest.mark.developer("needs DEVELOPER=1", "uses dev-fast-reconnect") @pytest.mark.openchannel('v2') @pytest.mark.openchannel('v1') @@ -1279,8 +1269,8 @@ def test_funding_reorg_private(node_factory, bitcoind): wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['status'] == ['{}_AWAITING_LOCKIN:Funding needs 1 more confirmations to be ready.'.format(daemon)]) bitcoind.generate_block(1) # height 107 - l1.wait_channel_active('106x1x0') - l2.wait_channel_active('106x1x0') + l1.wait_local_channel_active('106x1x0') + l2.wait_local_channel_active('106x1x0') l1.stop() # Create a fork that changes short_channel_id from 106x1x0 to 108x1x0 @@ -1293,13 +1283,11 @@ def test_funding_reorg_private(node_factory, bitcoind): r'Got depth change .->{} for .* REORG'.format(0)]) # New one should replace old. - wait_for(lambda: chan_active(l2, '108x1x0', True)) - assert l2.rpc.listchannels('106x1x0')['channels'] == [] + wait_for(lambda: l2.is_local_channel_active('108x1x0')) + assert [c for c in l2.rpc.listpeerchannels()['channels'] if c['short_channel_id'] == '106x1x0'] == [] l1.rpc.close(l2.info['id']) bitcoind.generate_block(1, True) - l1.daemon.wait_for_log(r'Deleting channel') - l2.daemon.wait_for_log(r'Deleting channel') @pytest.mark.developer("needs DEVELOPER=1", "uses --dev-fast-reconnect") @@ -1317,8 +1305,8 @@ def test_funding_reorg_remote_lags(node_factory, bitcoind): l1.rpc.fundchannel(l2.info['id'], "all") bitcoind.generate_block(5) # heights 103 - 107 - l1.wait_channel_active('103x1x0') - l2.wait_channel_active('103x1x0') + l1.wait_local_channel_active('103x1x0') + l2.wait_local_channel_active('103x1x0') # Make l2 temporary blind for blocks > 107 def no_more_blocks(req): @@ -1338,8 +1326,8 @@ def no_more_blocks(req): # Unblinding l2 brings it back in sync, restarts channeld and sends its announce sig l2.daemon.rpcproxy.mock_rpc('getblockhash', None) - wait_for(lambda: chan_active(l2, '104x1x0', True)) - assert l2.rpc.listchannels('103x1x0')['channels'] == [] + wait_for(lambda: l2.is_local_channel_active('104x1x0')) + assert [c for c in l2.rpc.listpeerchannels()['channels'] if c['short_channel_id'] == '103x1x0'] == [] wait_for(lambda: only_one(l2.rpc.listpeerchannels()['channels'])['status'] == [ 'CHANNELD_NORMAL:Reconnected, and reestablished.', diff --git a/tests/test_pay.py b/tests/test_pay.py index ece2734130af..644f04627b39 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -437,7 +437,7 @@ def test_payment_success_persistence(node_factory, bitcoind, executor): assert len(payments) == 1 and payments[0]['status'] == 'complete' assert len(invoices) == 1 and invoices[0]['status'] == 'paid' - l1.wait_channel_active(chanid) + l1.wait_local_channel_active(chanid) # A duplicate should succeed immediately (nop) and return correct preimage. preimage = l1.dev_pay( @@ -4001,13 +4001,13 @@ def test_mpp_waitblockheight_routehint_conflict(node_factory, bitcoind, executor l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1l2, _ = l1.fundchannel(l2, 10**7, announce_channel=True) l2.rpc.connect(l3.info['id'], 'localhost', l3.port) - l2.fundchannel(l3, 10**7, announce_channel=False) + l2l3, _ = l2.fundchannel(l3, 10**7, announce_channel=False) mine_funding_to_announce(bitcoind, [l1, l2, l3]) # Wait for l3 to learn about l1->l2, otherwise it will think # l2 is a deadend and not add it to the routehint. - wait_for(lambda: len(l3.rpc.listchannels(l1l2)['channels']) >= 2) + l3.wait_channel_active(l1l2) # Now make the l1 payer stop receiving blocks. def no_more_blocks(req): @@ -4019,7 +4019,11 @@ def no_more_blocks(req): bitcoind.generate_block(2) sync_blockheight(bitcoind, [l3]) + # FIXME: routehint currently requires channels in gossip store + l3.wait_channel_active(l2l3) + inv = l3.rpc.invoice(Millisatoshi(2 * 10000 * 1000), 'i', 'i', exposeprivatechannels=True)['bolt11'] + assert 'routes' in l3.rpc.decodepay(inv) # Have l1 pay l3 def pay(l1, inv): @@ -4869,12 +4873,15 @@ def test_routehint_tous(node_factory, bitcoind): # Existence of l1 makes l3 use l2 for routehint (otherwise it sees deadend) l1, l2 = node_factory.line_graph(2, wait_for_announce=True) + scid12 = first_scid(l1, l2) l3 = node_factory.get_node() l3.rpc.connect(l2.info['id'], 'localhost', l2.port) scid23, _ = l2.fundchannel(l3, 1000000, announce_channel=False) # Make sure l3 sees l1->l2 channel. - wait_for(lambda: l3.rpc.listnodes(l1.info['id'])['nodes'] != []) + l3.wait_channel_active(scid12) + # FIXME: Routehint code currently relies on private gossip in store! + l3.wait_channel_active(scid23) inv = l3.rpc.invoice(10, "test", "test")['bolt11'] decoded = l3.rpc.decodepay(inv) assert(only_one(only_one(decoded['routes']))['short_channel_id'] @@ -5352,7 +5359,7 @@ def test_pay_routehint_minhtlc(node_factory, bitcoind): l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True) l4 = node_factory.get_node() - l3.fundchannel(l4, announce_channel=False) + scid34, _ = l3.fundchannel(l4, announce_channel=False) # l2->l3 required htlc of at least 1sat scid = only_one(l2.rpc.setchannel(l3.info['id'], htlcmin=1000)['channels'])['short_channel_id'] @@ -5363,6 +5370,8 @@ def test_pay_routehint_minhtlc(node_factory, bitcoind): # And make sure l1 knows that l2->l3 has htlcmin 1000 wait_for(lambda: l1.rpc.listchannels(scid)['channels'][0]['htlc_minimum_msat'] == Millisatoshi(1000)) + # FIXME: Routehint code currently relies on private gossip in store! + l4.wait_channel_active(scid34) inv = l4.rpc.invoice(100000, "inv", "inv") assert only_one(l1.rpc.decodepay(inv['bolt11'])['routes']) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 52d246bc2cd0..7ddb151625f2 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2087,6 +2087,8 @@ def test_coin_movement_notices(node_factory, bitcoind, chainparams): # send a payment (originator) inv = l1.rpc.invoice(amount // 2, "second", "desc") payment_hash21 = inv['payment_hash'] + # Make sure previous completely settled + wait_for(lambda: only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['htlcs'] == []) route = l2.rpc.getroute(l1.info['id'], amount // 2, 1)['route'] l2.rpc.sendpay(route, payment_hash21, payment_secret=inv['payment_secret']) l2.rpc.waitsendpay(payment_hash21)