vincenzopalazzo
/
rgb-cln
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

connectd: use shutdown() not close() on TCP sockets for dev-disconnect. 

close() is allowed to lose data, and I saw this in CI:

```
2023-10-22T05:12:36.6576005Z ____________________________ test_permfail_htlc_out ____________________________
2023-10-22T05:12:36.6608511Z [gw2] linux -- Python 3.8.18 /home/runner/.cache/pypoetry/virtualenvs/cln-meta-project-AqJ9wMix-py3.8/bin/python
2023-10-22T05:12:36.6611663Z 
2023-10-22T05:12:36.6614768Z node_factory = <pyln.testing.utils.NodeFactory object at 0x7f381039a5e0>
2023-10-22T05:12:36.6623694Z bitcoind = <pyln.testing.utils.BitcoinD object at 0x7f38103c0400>
2023-10-22T05:12:36.6627092Z executor = <concurrent.futures.thread.ThreadPoolExecutor object at 0x7f38103c0ee0>
2023-10-22T05:12:36.6627701Z 
2023-10-22T05:12:36.6628051Z     def test_permfail_htlc_out(node_factory, bitcoind, executor):
2023-10-22T05:12:36.6631192Z         # Test case where we fail with unsettled outgoing HTLC.
2023-10-22T05:12:36.6634154Z         disconnects = ['+WIRE_REVOKE_AND_ACK', 'permfail']
2023-10-22T05:12:36.6635106Z         l1 = node_factory.get_node(options={'dev-no-reconnect': None})
2023-10-22T05:12:36.6637321Z         # Feerates identical so we don't get gratuitous commit to update them
2023-10-22T05:12:36.6642691Z         l2 = node_factory.get_node(disconnect=disconnects,
2023-10-22T05:12:36.6644734Z                                    feerates=(7500, 7500, 7500, 7500))
2023-10-22T05:12:36.6647205Z     
2023-10-22T05:12:36.6649671Z         l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
2023-10-22T05:12:36.6650460Z         l2.daemon.wait_for_log('Handed peer, entering loop')
2023-10-22T05:12:36.6654865Z         l2.fundchannel(l1, 10**6)
2023-10-22T05:12:36.6655305Z     
2023-10-22T05:12:36.6657810Z         # This will fail at l2's end.
2023-10-22T05:12:36.6660554Z         t = executor.submit(l2.pay, l1, 200000000)
2023-10-22T05:12:36.6662947Z     
2023-10-22T05:12:36.6665147Z         l2.daemon.wait_for_log('dev_disconnect permfail')
2023-10-22T05:12:36.6668530Z         l2.wait_for_channel_onchain(l1.info['id'])
2023-10-22T05:12:36.6671588Z         bitcoind.generate_block(1)
2023-10-22T05:12:36.6674510Z >       l1.daemon.wait_for_log('Their unilateral tx, old commit point')
2023-10-22T05:12:36.6675001Z 
2023-10-22T05:12:36.6675212Z tests/test_closing.py:3027: 
...
2023-10-22T05:12:36.8784390Z lightningd-2 2023-10-22T04:41:04.448Z DEBUG   0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-connectd: dev_disconnect: +WIRE_REVOKE_AND_ACK (WIRE_REVOKE_AND_ACK)
2023-10-22T05:12:36.8786260Z lightningd-2 2023-10-22T04:41:04.452Z INFO    0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-channeld-chan#1: Peer connection lost
2023-10-22T05:12:36.8788076Z lightningd-2 2023-10-22T04:41:04.453Z DEBUG   0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-channeld-chan#1: Status closed, but not exited. Killing
2023-10-22T05:12:36.8789915Z lightningd-1 2023-10-22T04:41:04.454Z INFO    022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: Peer connection lost
```

Note that l1 doesn't receive WIRE_REVOKE_AND_ACK!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2023-10-23 11:35:39 +10:30
parent 93189724dd
commit 1d61edfe0c
1 changed files with 8 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
connectd/multiplex.c
View File

@ -407,6 +407,12 @@ static bool is_urgent(enum peer_wire type)
return false;
}
/* io_sock_shutdown, but in format suitable for an io_plan callback */
static struct io_plan *io_sock_shutdown_cb(struct io_conn *conn, struct peer *unused)
{
return io_sock_shutdown(conn);
}
static struct io_plan *encrypt_and_send(struct peer *peer,
const u8 *msg TAKES,
struct io_plan *(*next)
@ -423,7 +429,8 @@ static struct io_plan *encrypt_and_send(struct peer *peer,
case DEV_DISCONNECT_AFTER:
/* Disallow reads from now on */
peer->dev_read_enabled = false;
next = (void *)io_close_cb;
/* Using io_close here can lose the data we're about to send! */
next = io_sock_shutdown_cb;
break;
case DEV_DISCONNECT_BLACKHOLE:
/* Disable both reads and writes from now on */