dev_disconnect: remove @ marker.

Once connectd is doing this, we can't close as soon as we send,
and in fact we can't do 'fail write' either.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/crypto_sync.c

@@ -29,8 +29,6 @@ void sync_crypto_write(struct per_peer_state *pps, const void *msg TAKES)
 	case DEV_DISCONNECT_BEFORE:
 		dev_sabotage_fd(pps->peer_fd, true);
 		peer_failed_connection_lost();
-	case DEV_DISCONNECT_DROPPKT:
-		enc = tal_free(enc); /* FALL THRU */
 	case DEV_DISCONNECT_AFTER:
 		post_sabotage = true;
 		post_close = true;

common/dev_disconnect.h

@@ -7,12 +7,10 @@
 enum dev_disconnect {
 	/* Do nothing. */
 	DEV_DISCONNECT_NORMAL = '=',
-	/* Close connection before sending packet (and fail write). */
+	/* Close connection before sending packet. */
 	DEV_DISCONNECT_BEFORE = '-',
 	/* Close connection after sending packet. */
 	DEV_DISCONNECT_AFTER = '+',
-	/* Close connection after dropping packet. */
-	DEV_DISCONNECT_DROPPKT = '@',
 	/* Swallow all writes from now on, and do no more reads. */
 	DEV_DISCONNECT_BLACKHOLE = '0',
 	/* Don't use connection after sending packet, but don't close. */

connectd/peer_exchange_initmsg.c

@@ -204,8 +204,6 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn,
 	case DEV_DISCONNECT_BEFORE:
 		dev_sabotage_fd(io_conn_fd(conn), true);
 		break;
-	case DEV_DISCONNECT_DROPPKT:
-		peer->msg = tal_free(peer->msg); /* FALL THRU */
 	case DEV_DISCONNECT_AFTER:
 		next = peer_write_postclose;
 		break;

tests/test_closing.py

@@ -312,7 +312,6 @@ def test_closing_different_fees(node_factory, bitcoind, executor):
 @pytest.mark.developer("needs DEVELOPER=1")
 def test_closing_negotiation_reconnect(node_factory, bitcoind):
     disconnects = ['-WIRE_CLOSING_SIGNED',
-                   '@WIRE_CLOSING_SIGNED',
                    '+WIRE_CLOSING_SIGNED']
     l1, l2 = node_factory.line_graph(2, opts=[{'disconnect': disconnects,
                                                'may_reconnect': True},
@@ -1956,7 +1955,7 @@ def test_onchain_dust_out(node_factory, bitcoind, executor):
     coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py')
 
     # HTLC 1->2, 1 fails after it's irrevocably committed
-    disconnects = ['@WIRE_REVOKE_AND_ACK', 'permfail']
+    disconnects = ['-WIRE_REVOKE_AND_ACK', 'permfail']
     # Feerates identical so we don't get gratuitous commit to update them
     l1, l2 = node_factory.line_graph(2,
                                      opts=[{'disconnect': disconnects,
@@ -3481,13 +3480,13 @@ def test_htlc_rexmit_while_closing(node_factory, executor):
 @pytest.mark.developer("needs dev_disconnect")
 def test_you_forgot_closed_channel(node_factory, executor):
     """Ideally you'd keep talking to us about closed channels: simple"""
-    disconnects = ['@WIRE_CLOSING_SIGNED']
+    disconnects = ['xWIRE_CLOSING_SIGNED']
 
     l1, l2 = node_factory.line_graph(2, opts=[{'may_reconnect': True,
-                                               'dev-no-reconnect': None},
-                                              {'may_reconnect': True,
                                                'dev-no-reconnect': None,
-                                               'disconnect': disconnects}])
+                                               'disconnect': disconnects},
+                                              {'may_reconnect': True,
+                                               'dev-no-reconnect': None}])
 
     l1.pay(l2, 200000)
 
@@ -3498,6 +3497,7 @@ def test_you_forgot_closed_channel(node_factory, executor):
     assert only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])['state'] == 'CLOSINGD_SIGEXCHANGE'
 
     # l1 reconnects, it should succeed.
+    l1.rpc.disconnect(l2.info['id'], force=True)
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     fut.result(TIMEOUT)
 
@@ -3505,13 +3505,13 @@ def test_you_forgot_closed_channel(node_factory, executor):
 @pytest.mark.developer("needs dev_disconnect")
 def test_you_forgot_closed_channel_onchain(node_factory, bitcoind, executor):
     """Ideally you'd keep talking to us about closed channels: even if close is mined"""
-    disconnects = ['@WIRE_CLOSING_SIGNED']
+    disconnects = ['xWIRE_CLOSING_SIGNED']
 
     l1, l2 = node_factory.line_graph(2, opts=[{'may_reconnect': True,
-                                               'dev-no-reconnect': None},
-                                              {'may_reconnect': True,
                                                'dev-no-reconnect': None,
-                                               'disconnect': disconnects}])
+                                               'disconnect': disconnects},
+                                              {'may_reconnect': True,
+                                               'dev-no-reconnect': None}])
 
     l1.pay(l2, 200000)
 
@@ -3533,6 +3533,8 @@ def test_you_forgot_closed_channel_onchain(node_factory, bitcoind, executor):
     wait_for(lambda: only_one(only_one(l2.rpc.listpeers()['peers'])['channels'])['state'] == 'ONCHAIN')
 
     # l1 reconnects, it should succeed.
+    # l1 will disconnect once it sees block
+    wait_for(lambda: only_one(l1.rpc.listpeers()['peers'])['connected'] is False)
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     fut.result(TIMEOUT)
 

tests/test_connection.py

@@ -289,7 +289,6 @@ def test_channel_abandon(node_factory, bitcoind):
 def test_disconnect(node_factory):
     # These should all make us fail
     disconnects = ['-WIRE_INIT',
-                   '@WIRE_INIT',
                    '+WIRE_INIT']
     l1 = node_factory.get_node(disconnect=disconnects)
     l2 = node_factory.get_node()
@@ -298,8 +297,6 @@ def test_disconnect(node_factory):
         l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     with pytest.raises(RpcError):
         l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-    with pytest.raises(RpcError):
-        l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
 
     # Should have 3 connect fails.
     for d in disconnects:
@@ -317,22 +314,16 @@ def test_disconnect(node_factory):
 def test_disconnect_opener(node_factory):
     # Now error on opener side during channel open.
     disconnects = ['-WIRE_OPEN_CHANNEL',
-                   '@WIRE_OPEN_CHANNEL',
                    '+WIRE_OPEN_CHANNEL',
-                   '-WIRE_FUNDING_CREATED',
+                   '-WIRE_FUNDING_CREATED']
-                   '@WIRE_FUNDING_CREATED']
     if EXPERIMENTAL_DUAL_FUND:
         disconnects = ['-WIRE_OPEN_CHANNEL2',
-                       '@WIRE_OPEN_CHANNEL2',
                        '+WIRE_OPEN_CHANNEL2',
                        '-WIRE_TX_ADD_INPUT',
-                       '@WIRE_TX_ADD_INPUT',
                        '+WIRE_TX_ADD_INPUT',
                        '-WIRE_TX_ADD_OUTPUT',
-                       '@WIRE_TX_ADD_OUTPUT',
                        '+WIRE_TX_ADD_OUTPUT',
                        '-WIRE_TX_COMPLETE',
-                       '@WIRE_TX_COMPLETE',
                        '+WIRE_TX_COMPLETE']
 
     l1 = node_factory.get_node(disconnect=disconnects)
@@ -361,14 +352,11 @@ def test_disconnect_opener(node_factory):
 def test_disconnect_fundee(node_factory):
     # Now error on fundee side during channel open.
     disconnects = ['-WIRE_ACCEPT_CHANNEL',
-                   '@WIRE_ACCEPT_CHANNEL',
                    '+WIRE_ACCEPT_CHANNEL']
     if EXPERIMENTAL_DUAL_FUND:
         disconnects = ['-WIRE_ACCEPT_CHANNEL2',
-                       '@WIRE_ACCEPT_CHANNEL2',
                        '+WIRE_ACCEPT_CHANNEL2',
                        '-WIRE_TX_COMPLETE',
-                       '@WIRE_TX_COMPLETE',
                        '+WIRE_TX_COMPLETE']
 
     l1 = node_factory.get_node()
@@ -397,16 +385,12 @@ def test_disconnect_fundee(node_factory):
 def test_disconnect_fundee_v2(node_factory):
     # Now error on fundee side during channel open, with them funding
     disconnects = ['-WIRE_ACCEPT_CHANNEL2',
-                   '@WIRE_ACCEPT_CHANNEL2',
                    '+WIRE_ACCEPT_CHANNEL2',
                    '-WIRE_TX_ADD_INPUT',
-                   '@WIRE_TX_ADD_INPUT',
                    '+WIRE_TX_ADD_INPUT',
                    '-WIRE_TX_ADD_OUTPUT',
-                   '@WIRE_TX_ADD_OUTPUT',
                    '+WIRE_TX_ADD_OUTPUT',
                    '-WIRE_TX_COMPLETE',
-                   '@WIRE_TX_COMPLETE',
                    '+WIRE_TX_COMPLETE']
 
     l1 = node_factory.get_node()
@@ -440,9 +424,9 @@ def test_disconnect_fundee_v2(node_factory):
 def test_disconnect_half_signed(node_factory):
     # Now, these are the corner cases.  Fundee sends funding_signed,
     # but opener doesn't receive it.
-    disconnects = ['@WIRE_FUNDING_SIGNED']
+    disconnects = ['-WIRE_FUNDING_SIGNED']
     if EXPERIMENTAL_DUAL_FUND:
-        disconnects = ['@WIRE_COMMITMENT_SIGNED']
+        disconnects = ['-WIRE_COMMITMENT_SIGNED']
     l1 = node_factory.get_node()
     l2 = node_factory.get_node(disconnect=disconnects)
 
@@ -567,7 +551,7 @@ def test_reconnect_no_update(node_factory, executor, bitcoind):
     reconnects. See comments for details.
 
     """
-    disconnects = ["@WIRE_FUNDING_LOCKED", "@WIRE_SHUTDOWN"]
+    disconnects = ["-WIRE_FUNDING_LOCKED", "-WIRE_SHUTDOWN"]
     # Allow bad gossip because it might receive WIRE_CHANNEL_UPDATE before
     # announcement of the disconnection
     l1 = node_factory.get_node(may_reconnect=True, allow_bad_gossip=True)
@@ -591,7 +575,7 @@ def test_reconnect_no_update(node_factory, executor, bitcoind):
 
     # For closingd reconnection
     l1.daemon.start()
-    # Close will trigger the @WIRE_SHUTDOWN and we then wait for the
+    # Close will trigger the -WIRE_SHUTDOWN and we then wait for the
     # automatic reconnection to trigger the retransmission.
     l1.rpc.close(l2.info['id'], 0)
     l2.daemon.wait_for_log(r"channeld.* Retransmitting funding_locked for channel")
@@ -645,7 +629,6 @@ def test_connect_stresstest(node_factory, executor):
 def test_reconnect_normal(node_factory):
     # Should reconnect fine even if locked message gets lost.
     disconnects = ['-WIRE_FUNDING_LOCKED',
-                   '@WIRE_FUNDING_LOCKED',
                    '+WIRE_FUNDING_LOCKED']
     l1 = node_factory.get_node(disconnect=disconnects,
                                may_reconnect=True)
@@ -661,8 +644,7 @@ def test_reconnect_normal(node_factory):
 def test_reconnect_sender_add1(node_factory):
     # Fail after add is OK, will cause payment failure though.
     disconnects = ['-WIRE_UPDATE_ADD_HTLC-nocommit',
-                   '+WIRE_UPDATE_ADD_HTLC-nocommit',
+                   '+WIRE_UPDATE_ADD_HTLC-nocommit']
-                   '@WIRE_UPDATE_ADD_HTLC-nocommit']
 
     # Feerates identical so we don't get gratuitous commit to update them
     l1 = node_factory.get_node(disconnect=disconnects,
@@ -697,10 +679,8 @@ def test_reconnect_sender_add1(node_factory):
 @pytest.mark.openchannel('v2')
 def test_reconnect_sender_add(node_factory):
     disconnects = ['-WIRE_COMMITMENT_SIGNED',
-                   '@WIRE_COMMITMENT_SIGNED',
                    '+WIRE_COMMITMENT_SIGNED',
                    '-WIRE_REVOKE_AND_ACK',
-                   '@WIRE_REVOKE_AND_ACK',
                    '+WIRE_REVOKE_AND_ACK']
     if EXPERIMENTAL_DUAL_FUND:
         disconnects = ['=WIRE_COMMITMENT_SIGNED'] + disconnects
@@ -733,10 +713,8 @@ def test_reconnect_sender_add(node_factory):
 @pytest.mark.openchannel('v2')
 def test_reconnect_receiver_add(node_factory):
     disconnects = ['-WIRE_COMMITMENT_SIGNED',
-                   '@WIRE_COMMITMENT_SIGNED',
                    '+WIRE_COMMITMENT_SIGNED',
                    '-WIRE_REVOKE_AND_ACK',
-                   '@WIRE_REVOKE_AND_ACK',
                    '+WIRE_REVOKE_AND_ACK']
 
     if EXPERIMENTAL_DUAL_FUND:
@@ -767,14 +745,11 @@ def test_reconnect_receiver_fulfill(node_factory):
     # Ordering matters: after +WIRE_UPDATE_FULFILL_HTLC, channeld
     # will continue and try to send WIRE_COMMITMENT_SIGNED: if
     # that's the next failure, it will do two in one run.
-    disconnects = ['@WIRE_UPDATE_FULFILL_HTLC',
+    disconnects = ['+WIRE_UPDATE_FULFILL_HTLC',
-                   '+WIRE_UPDATE_FULFILL_HTLC',
                    '-WIRE_UPDATE_FULFILL_HTLC',
                    '-WIRE_COMMITMENT_SIGNED',
-                   '@WIRE_COMMITMENT_SIGNED',
                    '+WIRE_COMMITMENT_SIGNED',
                    '-WIRE_REVOKE_AND_ACK',
-                   '@WIRE_REVOKE_AND_ACK',
                    '+WIRE_REVOKE_AND_ACK']
     l1 = node_factory.get_node(may_reconnect=True)
     l2 = node_factory.get_node(disconnect=disconnects,
@@ -801,7 +776,6 @@ def test_reconnect_receiver_fulfill(node_factory):
 @pytest.mark.openchannel('v2')
 def test_shutdown_reconnect(node_factory):
     disconnects = ['-WIRE_SHUTDOWN',
-                   '@WIRE_SHUTDOWN',
                    '+WIRE_SHUTDOWN']
     l1 = node_factory.get_node(disconnect=disconnects,
                                may_reconnect=True)
@@ -1802,7 +1776,7 @@ def test_multifunding_disconnect(node_factory):
     '''
     Test disconnection during multifundchannel
     '''
-    # TODO: Note that @WIRE_FUNDING_SIGNED does not
+    # TODO: Note that -WIRE_FUNDING_SIGNED does not
     # work.
     # See test_disconnect_half_signed.
     # If disconnected when the peer believes it sent
@@ -1812,9 +1786,7 @@ def test_multifunding_disconnect(node_factory):
     # never send it.
     disconnects = ["-WIRE_INIT",
                    "-WIRE_ACCEPT_CHANNEL",
-                   "@WIRE_ACCEPT_CHANNEL",
+                   "+WIRE_ACCEPT_CHANNEL"]
-                   "+WIRE_ACCEPT_CHANNEL",
-                   "-WIRE_FUNDING_SIGNED"]
     l1 = node_factory.get_node()
     l2 = node_factory.get_node(disconnect=disconnects)
     l3 = node_factory.get_node()
@@ -1833,7 +1805,7 @@ def test_multifunding_disconnect(node_factory):
             l1.rpc.multifundchannel(destinations)
 
     # TODO: failing at the fundchannel_complete phase
-    # (@WIRE_FUNDING_SIGNED +@WIRE_FUNDING_SIGNED)
+    # (-WIRE_FUNDING_SIGNED +-WIRE_FUNDING_SIGNED)
     # leaves the peer (l2 in this case) in a state
     # where it is waiting for an incoming channel,
     # even though we no longer have a channel going to

tests/test_misc.py

@@ -316,7 +316,7 @@ def test_htlc_out_timeout(node_factory, bitcoind, executor):
     """Test that we drop onchain if the peer doesn't time out HTLC"""
 
     # HTLC 1->2, 1 fails after it's irrevocably committed, can't reconnect
-    disconnects = ['@WIRE_REVOKE_AND_ACK']
+    disconnects = ['-WIRE_REVOKE_AND_ACK']
     # Feerates identical so we don't get gratuitous commit to update them
     l1 = node_factory.get_node(disconnect=disconnects,
                                options={'dev-no-reconnect': None},
@@ -336,7 +336,7 @@ def test_htlc_out_timeout(node_factory, bitcoind, executor):
     executor.submit(l1.rpc.dev_pay, inv, use_shadow=False)
 
     # l1 will disconnect, and not reconnect.
-    l1.daemon.wait_for_log('dev_disconnect: @WIRE_REVOKE_AND_ACK')
+    l1.daemon.wait_for_log('dev_disconnect: -WIRE_REVOKE_AND_ACK')
 
     # Takes 6 blocks to timeout (cltv-final + 1), but we also give grace period of 1 block.
     # shadow route can add extra blocks!

tests/test_opening.py

@@ -508,7 +508,6 @@ def test_v2_rbf_multi(node_factory, bitcoind, chainparams):
 @pytest.mark.openchannel('v2')
 def test_rbf_reconnect_init(node_factory, bitcoind, chainparams):
     disconnects = ['-WIRE_INIT_RBF',
-                   '@WIRE_INIT_RBF',
                    '+WIRE_INIT_RBF']
 
     l1, l2 = node_factory.get_nodes(2,
@@ -559,7 +558,6 @@ def test_rbf_reconnect_init(node_factory, bitcoind, chainparams):
 @pytest.mark.openchannel('v2')
 def test_rbf_reconnect_ack(node_factory, bitcoind, chainparams):
     disconnects = ['-WIRE_ACK_RBF',
-                   '@WIRE_ACK_RBF',
                    '+WIRE_ACK_RBF']
 
     l1, l2 = node_factory.get_nodes(2,
@@ -611,13 +609,10 @@ def test_rbf_reconnect_ack(node_factory, bitcoind, chainparams):
 def test_rbf_reconnect_tx_construct(node_factory, bitcoind, chainparams):
     disconnects = ['=WIRE_TX_ADD_INPUT',  # Initial funding succeeds
                    '-WIRE_TX_ADD_INPUT',
-                   '@WIRE_TX_ADD_INPUT',
                    '+WIRE_TX_ADD_INPUT',
                    '-WIRE_TX_ADD_OUTPUT',
-                   '@WIRE_TX_ADD_OUTPUT',
                    '+WIRE_TX_ADD_OUTPUT',
                    '-WIRE_TX_COMPLETE',
-                   '@WIRE_TX_COMPLETE',
                    '+WIRE_TX_COMPLETE']
 
     l1, l2 = node_factory.get_nodes(2,
@@ -652,14 +647,14 @@ def test_rbf_reconnect_tx_construct(node_factory, bitcoind, chainparams):
                                excess_as_change=True)
 
     # Run through TX_ADD wires
-    for d in disconnects[1:-3]:
+    for d in disconnects[1:-2]:
         l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
         with pytest.raises(RpcError):
             l1.rpc.openchannel_bump(chan_id, chan_amount, initpsbt['psbt'])
         assert l1.rpc.getpeer(l2.info['id']) is not None
 
     # Now we finish off the completes failure check
-    for d in disconnects[-3:]:
+    for d in disconnects[-2:]:
         l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
         bump = l1.rpc.openchannel_bump(chan_id, chan_amount, initpsbt['psbt'])
         with pytest.raises(RpcError):
@@ -679,8 +674,6 @@ def test_rbf_reconnect_tx_sigs(node_factory, bitcoind, chainparams):
     disconnects = ['=WIRE_TX_SIGNATURES',  # Initial funding succeeds
                    '-WIRE_TX_SIGNATURES',  # When we send tx-sigs, RBF
                    '=WIRE_TX_SIGNATURES',  # When we reconnect
-                   '@WIRE_TX_SIGNATURES',  # When we RBF again
-                   '=WIRE_TX_SIGNATURES',  # When we reconnect
                    '+WIRE_TX_SIGNATURES']  # When we RBF again
 
     l1, l2 = node_factory.get_nodes(2,
@@ -753,52 +746,16 @@ def test_rbf_reconnect_tx_sigs(node_factory, bitcoind, chainparams):
     l1.daemon.wait_for_log('peer_in WIRE_CHANNEL_REESTABLISH')
     l1.daemon.wait_for_log('peer_in WIRE_TX_SIGNATURES')
 
-    # Now we initiate the RBF
+    # 2nd RBF
     bump = l1.rpc.openchannel_bump(chan_id, chan_amount, initpsbt['psbt'],
                                    funding_feerate=next_feerate)
     update = l1.rpc.openchannel_update(chan_id, bump['psbt'])
-
-    # Sign our inputs, and continue
     signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt']
 
     # Second time we error after we send our sigs
     with pytest.raises(RpcError, match='Owning subdaemon dualopend died'):
         l1.rpc.openchannel_signed(chan_id, signed_psbt)
 
-    # We reconnect and try again. feerate should have bumped
-    rate = int(find_next_feerate(l1, l2)[:-5])
-    # We bump the feerate to beat the min-relay fee
-    next_feerate = '{}perkw'.format(rate * 2)
-
-    startweight = 42 + 172  # base weight, funding output
-    initpsbt = l1.rpc.utxopsbt(chan_amount, next_feerate, startweight,
-                               prev_utxos, reservedok=True,
-                               min_witness_weight=110,
-                               excess_as_change=True)
-
-    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-
-    # l2 gets our sigs and broadcasts them
-    l2.daemon.wait_for_log('peer_in WIRE_CHANNEL_REESTABLISH')
-    l2.daemon.wait_for_log('peer_in WIRE_TX_SIGNATURES')
-    l2.daemon.wait_for_log('sendrawtx exit 0')
-
-    # Wait until we've done re-establish, if we try to
-    # RBF again too quickly, it'll fail since they haven't
-    # had time to process our sigs yet
-    l1.daemon.wait_for_log('peer_in WIRE_CHANNEL_REESTABLISH')
-    l1.daemon.wait_for_log('peer_in WIRE_TX_SIGNATURES')
-
-    # 3rd RBF
-    bump = l1.rpc.openchannel_bump(chan_id, chan_amount, initpsbt['psbt'],
-                                   funding_feerate=next_feerate)
-    update = l1.rpc.openchannel_update(chan_id, bump['psbt'])
-    signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt']
-
-    # Third time we error after we send our sigs
-    with pytest.raises(RpcError, match='Owning subdaemon dualopend died'):
-        l1.rpc.openchannel_signed(chan_id, signed_psbt)
-
     # l2 gets our sigs
     l2.daemon.wait_for_log('peer_in WIRE_TX_SIGNATURES')
     l2.daemon.wait_for_log('sendrawtx exit 0')