From 6ceec17943f34144229f660a4d4371406e62561f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Oct 2017 20:38:50 +1030 Subject: [PATCH] dev_disconnect: make commit suppression a "-nocommit" modifier. Useful if we want to drop & suppress, for example. We change '=' to mean do nothing to the packet. We use this to clean up the test_reconnect_sender_add test. Signed-off-by: Rusty Russell --- common/crypto_sync.c | 1 - common/cryptomsg.c | 1 - common/dev_disconnect.c | 14 ++++++-- common/dev_disconnect.h | 4 +-- tests/test_lightningd.py | 72 ++++++++++++++++++++++------------------ 5 files changed, 51 insertions(+), 41 deletions(-) diff --git a/common/crypto_sync.c b/common/crypto_sync.c index f5ea8b821..e3b53f54a 100644 --- a/common/crypto_sync.c +++ b/common/crypto_sync.c @@ -28,7 +28,6 @@ bool sync_crypto_write(struct crypto_state *cs, int fd, const void *msg TAKES) case DEV_DISCONNECT_BLACKHOLE: dev_blackhole_fd(fd); break; - case DEV_DISCONNECT_SUPPRESS_COMMIT: case DEV_DISCONNECT_NORMAL: break; } diff --git a/common/cryptomsg.c b/common/cryptomsg.c index 617612d21..b23af5b4a 100644 --- a/common/cryptomsg.c +++ b/common/cryptomsg.c @@ -355,7 +355,6 @@ struct io_plan *peer_write_message(struct io_conn *conn, case DEV_DISCONNECT_BLACKHOLE: dev_blackhole_fd(io_conn_fd(conn)); break; - case DEV_DISCONNECT_SUPPRESS_COMMIT: case DEV_DISCONNECT_NORMAL: break; } diff --git a/common/dev_disconnect.c b/common/dev_disconnect.c index 082f2a920..3c2ecefe2 100644 --- a/common/dev_disconnect.c +++ b/common/dev_disconnect.c @@ -15,6 +15,7 @@ static int dev_disconnect_fd = -1; static char dev_disconnect_line[200]; static int dev_disconnect_count, dev_disconnect_len; +static bool dev_disconnect_nocommit; bool dev_suppress_commit; @@ -32,6 +33,13 @@ void dev_disconnect_init(int fd) dev_disconnect_line[r] = '\n'; dev_disconnect_len = strcspn(dev_disconnect_line, "\n"); dev_disconnect_line[dev_disconnect_len] = '\0'; + if (strends(dev_disconnect_line, "-nocommit")) { + dev_disconnect_line[strlen(dev_disconnect_line) + - strlen("-nocommit")] = '\0'; + dev_disconnect_nocommit = true; + } else + dev_disconnect_nocommit = false; + asterisk = strchr(dev_disconnect_line, '*'); if (asterisk) { dev_disconnect_count = atoi(asterisk+1); @@ -59,9 +67,9 @@ enum dev_disconnect dev_disconnect(int pkt_type) assert(dev_disconnect_fd != -1); lseek(dev_disconnect_fd, dev_disconnect_len+1, SEEK_CUR); - status_trace("dev_disconnect: %s", dev_disconnect_line); - - if (dev_disconnect_line[0] == DEV_DISCONNECT_SUPPRESS_COMMIT) + status_trace("dev_disconnect: %s%s", dev_disconnect_line, + dev_disconnect_nocommit ? "-nocommit" : ""); + if (dev_disconnect_nocommit) dev_suppress_commit = true; return dev_disconnect_line[0]; } diff --git a/common/dev_disconnect.h b/common/dev_disconnect.h index 9b6a243d2..026c672f0 100644 --- a/common/dev_disconnect.h +++ b/common/dev_disconnect.h @@ -5,7 +5,7 @@ enum dev_disconnect { /* Do nothing. */ - DEV_DISCONNECT_NORMAL = 0, + DEV_DISCONNECT_NORMAL = '=', /* Close connection before sending packet (and fail write). */ DEV_DISCONNECT_BEFORE = '-', /* Close connection after sending packet. */ @@ -14,8 +14,6 @@ enum dev_disconnect { DEV_DISCONNECT_DROPPKT = '@', /* Swallow all writes from now on, and do no more reads. */ DEV_DISCONNECT_BLACKHOLE = '0', - /* Disable commit timer after sending this. */ - DEV_DISCONNECT_SUPPRESS_COMMIT = '_' }; /* Force a close fd before or after a certain packet type */ diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 1d22dade3..850510214 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -706,8 +706,8 @@ class LightningDTests(BaseLightningDTests): def test_penalty_inhtlc(self): """Test penalty transaction with an incoming HTLC""" # We suppress each one after first commit; HTLC gets added not fulfilled. - l1 = self.node_factory.get_node(disconnect=['_WIRE_COMMITMENT_SIGNED'], may_fail=True) - l2 = self.node_factory.get_node(disconnect=['_WIRE_COMMITMENT_SIGNED']) + l1 = self.node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED-nocommit'], may_fail=True) + l2 = self.node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED-nocommit']) l1.rpc.connect(l2.info['id'], 'localhost:{}'.format(l2.info['port'])) self.fund_channel(l1, l2, 10**6) @@ -716,8 +716,8 @@ class LightningDTests(BaseLightningDTests): t = self.pay(l1,l2,100000000,async=True) # They should both have commitments blocked now. - l1.daemon.wait_for_log('_WIRE_COMMITMENT_SIGNED') - l2.daemon.wait_for_log('_WIRE_COMMITMENT_SIGNED') + l1.daemon.wait_for_log('=WIRE_COMMITMENT_SIGNED-nocommit') + l2.daemon.wait_for_log('=WIRE_COMMITMENT_SIGNED-nocommit') # Make sure l1 got l2's commitment to the HTLC, and sent to master. l1.daemon.wait_for_log('UPDATE WIRE_CHANNEL_GOT_COMMITSIG') @@ -765,8 +765,8 @@ class LightningDTests(BaseLightningDTests): def test_penalty_outhtlc(self): """Test penalty transaction with an outgoing HTLC""" # First we need to get funds to l2, so suppress after second. - l1 = self.node_factory.get_node(disconnect=['_WIRE_COMMITMENT_SIGNED*3'], may_fail=True) - l2 = self.node_factory.get_node(disconnect=['_WIRE_COMMITMENT_SIGNED*3']) + l1 = self.node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED*3-nocommit'], may_fail=True) + l2 = self.node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED*3-nocommit']) l1.rpc.connect(l2.info['id'], 'localhost:{}'.format(l2.info['port'])) self.fund_channel(l1, l2, 10**6) @@ -774,8 +774,8 @@ class LightningDTests(BaseLightningDTests): # Move some across to l2. self.pay(l1,l2,200000000) - assert not l1.daemon.is_in_log('_WIRE_COMMITMENT_SIGNED') - assert not l2.daemon.is_in_log('_WIRE_COMMITMENT_SIGNED') + assert not l1.daemon.is_in_log('=WIRE_COMMITMENT_SIGNED') + assert not l2.daemon.is_in_log('=WIRE_COMMITMENT_SIGNED') # Now, this will get stuck due to l1 commit being disabled.. t = self.pay(l2,l1,100000000,async=True) @@ -784,8 +784,8 @@ class LightningDTests(BaseLightningDTests): l1.daemon.wait_for_log('peer_in WIRE_COMMITMENT_SIGNED') # They should both have commitments blocked now. - l1.daemon.wait_for_log('dev_disconnect: _WIRE_COMMITMENT_SIGNED') - l2.daemon.wait_for_log('dev_disconnect: _WIRE_COMMITMENT_SIGNED') + l1.daemon.wait_for_log('dev_disconnect: =WIRE_COMMITMENT_SIGNED') + l2.daemon.wait_for_log('dev_disconnect: =WIRE_COMMITMENT_SIGNED') # Take our snapshot. tx = l1.rpc.dev_sign_last_tx(l2.info['id'])['tx'] @@ -1279,12 +1279,34 @@ class LightningDTests(BaseLightningDTests): self.fund_channel(l1, l2, 10**6) - def test_reconnect_sender_add(self): + def test_reconnect_sender_add1(self): # Fail after add is OK, will cause payment failure though. - disconnects = ['-WIRE_UPDATE_ADD_HTLC', - '@WIRE_UPDATE_ADD_HTLC', - '+WIRE_UPDATE_ADD_HTLC', - '-WIRE_COMMITMENT_SIGNED', + disconnects = ['-WIRE_UPDATE_ADD_HTLC-nocommit', + '+WIRE_UPDATE_ADD_HTLC-nocommit', + '@WIRE_UPDATE_ADD_HTLC-nocommit'] + + l1 = self.node_factory.get_node(disconnect=disconnects) + l2 = self.node_factory.get_node() + ret = l1.rpc.connect(l2.info['id'], 'localhost:{}'.format(l2.info['port'])) + + self.fund_channel(l1, l2, 10**6) + + amt = 200000000 + rhash = l2.rpc.invoice(amt, 'test_reconnect_sender_add1')['rhash'] + assert l2.rpc.listinvoice('test_reconnect_sender_add1')[0]['complete'] == False + + route = [ { 'msatoshi' : amt, 'id' : l2.info['id'], 'delay' : 5, 'channel': '1:1:1'} ] + + for i in range(0,len(disconnects)): + self.assertRaises(ValueError, l1.rpc.sendpay, to_json(route), rhash) + # Wait for reconnection. + l1.daemon.wait_for_log('Already have funding locked in') + + # This will send commit, so will reconnect as required. + l1.rpc.sendpay(to_json(route), rhash) + + def test_reconnect_sender_add(self): + disconnects = ['-WIRE_COMMITMENT_SIGNED', '@WIRE_COMMITMENT_SIGNED', '+WIRE_COMMITMENT_SIGNED', '-WIRE_REVOKE_AND_ACK', @@ -1301,27 +1323,11 @@ class LightningDTests(BaseLightningDTests): assert l2.rpc.listinvoice('testpayment')[0]['complete'] == False route = [ { 'msatoshi' : amt, 'id' : l2.info['id'], 'delay' : 5, 'channel': '1:1:1'} ] - # First time, it will fail because it doesn't send commit. - self.assertRaises(ValueError, l1.rpc.sendpay, to_json(route), rhash) - # Wait for reconnection. - l1.daemon.wait_for_log('Already have funding locked in') - - # These are *racy* whether they succeeds or not: does the commit timer - # fire before it tries reading and notices fd is closed? - for i in range(1,3): - try: - l1.rpc.sendpay(to_json(route), rhash) - assert l2.rpc.listinvoice('testpayment')[0]['complete'] == True - rhash = l2.rpc.invoice(amt, 'testpayment' + str(i))['rhash'] - except: - pass - # Wait for reconnection. - l1.daemon.wait_for_log('Already have funding locked in') # This will send commit, so will reconnect as required. l1.rpc.sendpay(to_json(route), rhash) # Should have printed this for every reconnect. - for i in range(3,len(disconnects)): + for i in range(0,len(disconnects)): l1.daemon.wait_for_log('Already have funding locked in') def test_reconnect_receiver_add(self): @@ -1503,7 +1509,7 @@ class LightningDTests(BaseLightningDTests): # mysteriously die while committing the first HTLC so we can # check that HTLCs reloaded from the DB work. l1 = self.node_factory.get_node() - l2 = self.node_factory.get_node(disconnect=['_WIRE_COMMITMENT_SIGNED']) + l2 = self.node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED-nocommit']) l1.rpc.connect(l2.info['id'], 'localhost:{}'.format(l2.info['port'])) # Neither node should have a channel open, they are just connected