From e6d56ff8ef40a4f71d774cb03a6938cd82ced701 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Sun, 21 Jan 2018 12:01:18 +0100 Subject: [PATCH 1/9] channeld: Send disabling channel_update on shutdown Sends a disable channel_update before issuing the shutdown message, gossipd will also take care to update others and not use for future routes. Signed-off-by: Christian Decker --- channeld/channel.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/channeld/channel.c b/channeld/channel.c index a601c342d703..a60f3f7b7e4d 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -716,6 +716,12 @@ static void maybe_send_shutdown(struct peer *peer) if (!peer->unsent_shutdown_scriptpubkey) return; + /* Send a disable channel_update so others don't try to route + * over us */ + msg = create_channel_update(peer, peer, true); + wire_sync_write(GOSSIP_FD, msg); + enqueue_peer_msg(peer, take(msg)); + msg = towire_shutdown(peer, &peer->channel_id, peer->unsent_shutdown_scriptpubkey); enqueue_peer_msg(peer, take(msg)); From 84cfb5127ccb4fde9d8ef504e1f3767c8b0ffe93 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Sun, 21 Jan 2018 12:05:34 +0100 Subject: [PATCH 2/9] channeld: Tell gossipd when we get a shutdown message from a peer Disabling the channel and enqueing the update for broadcast so we don't get forwarding requests from remote peers, and we don't try to ourselves. Signed-off-by: Christian Decker --- channeld/channel.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/channeld/channel.c b/channeld/channel.c index a60f3f7b7e4d..48984351f605 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -1528,7 +1528,10 @@ static void handle_pong(struct peer *peer, const u8 *pong) static void handle_peer_shutdown(struct peer *peer, const u8 *shutdown) { struct channel_id channel_id; - u8 *scriptpubkey; + u8 *scriptpubkey, *msg; + + msg = create_channel_update(peer, peer, true); + wire_sync_write(GOSSIP_FD, take(msg)); if (!fromwire_shutdown(peer, shutdown, NULL, &channel_id, &scriptpubkey)) peer_failed(PEER_FD, From 238e4c23e2301045664f6c17830aa790b917dc9b Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 23 Jan 2018 22:11:44 +0100 Subject: [PATCH 3/9] masterd: Peer has to know which direction an eventual channel is Signed-off-by: Christian Decker --- lightningd/peer_control.c | 2 ++ lightningd/peer_control.h | 3 +++ 2 files changed, 5 insertions(+) diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 4bf2f4e555fb..3d3ac9c6df0d 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -2091,6 +2091,8 @@ static bool peer_start_channeld(struct peer *peer, if (peer->ld->config.ignore_fee_limits) log_debug(peer->log, "Ignoring fee limits!"); + peer->direction = pubkey_cmp(&peer->ld->id, &peer->id) > 0; + initmsg = towire_channel_init(tmpctx, &get_chainparams(peer->ld) ->genesis_blockhash, diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index 1b8793566cdd..51f5003ed8dc 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -100,6 +100,9 @@ struct peer { struct changed_htlc *last_sent_commit; struct wallet_channel *channel; + + /* If we open a channel our direction will be this */ + u8 direction; }; static inline bool peer_can_add_htlc(const struct peer *peer) From cb7bf5e0dfbccff2807e943d68fa94db95011986 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 23 Jan 2018 22:13:19 +0100 Subject: [PATCH 4/9] gossip: Add message to enable and disable a channel Signed-off-by: Christian Decker --- gossipd/gossip.c | 9 +++++++++ gossipd/gossip_wire.csv | 5 +++++ lightningd/gossip_control.c | 1 + 3 files changed, 15 insertions(+) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index ec62d15ec6dd..5bb96443328f 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -1769,6 +1769,12 @@ static struct io_plan *handle_txout_reply(struct io_conn *conn, return daemon_conn_read_next(conn, &daemon->master); } +static struct io_plan *handle_disable_channel(struct io_conn *conn, + struct daemon *daemon, u8 *msg) +{ + return daemon_conn_read_next(conn, &daemon->master); +} + static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master) { struct daemon *daemon = container_of(master, struct daemon, master); @@ -1814,6 +1820,9 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master case WIRE_GOSSIP_GET_TXOUT_REPLY: return handle_txout_reply(conn, daemon, master->msg_in); + case WIRE_GOSSIP_DISABLE_CHANNEL: + return handle_disable_channel(conn, daemon, master->msg_in); + /* We send these, we don't receive them */ case WIRE_GOSSIPCTL_RELEASE_PEER_REPLY: case WIRE_GOSSIPCTL_RELEASE_PEER_REPLYFAIL: diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index 85ffa49d67f9..29b98cb83091 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -183,3 +183,8 @@ gossip_get_txout_reply,,short_channel_id,struct short_channel_id gossip_get_txout_reply,,len,u16 gossip_get_txout_reply,,outscript,len*u8 +# client->gossipd: Disable the channel matching the short_channel_id +gossip_disable_channel,3019 +gossip_disable_channel,,short_channel_id,struct short_channel_id +gossip_disable_channel,,direction,u8 +gossip_disable_channel,,active,bool diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 5712dc929fe3..9792cf193a4e 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -109,6 +109,7 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) case WIRE_GOSSIP_GET_UPDATE: case WIRE_GOSSIP_SEND_GOSSIP: case WIRE_GOSSIP_GET_TXOUT_REPLY: + case WIRE_GOSSIP_DISABLE_CHANNEL: /* This is a reply, so never gets through to here. */ case WIRE_GOSSIP_GET_UPDATE_REPLY: case WIRE_GOSSIP_GETNODES_REPLY: From 9608240935f64f4409491e54a3145397f44a5ee8 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 23 Jan 2018 22:14:53 +0100 Subject: [PATCH 5/9] gossip: Implement handler for enabling and disabling channels Signed-off-by: Christian Decker --- gossipd/gossip.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 5bb96443328f..0b0489969fbf 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -1772,6 +1772,85 @@ static struct io_plan *handle_txout_reply(struct io_conn *conn, static struct io_plan *handle_disable_channel(struct io_conn *conn, struct daemon *daemon, u8 *msg) { + tal_t *tmpctx = tal_tmpctx(msg); + struct short_channel_id scid; + u8 direction; + struct node_connection *nc; + bool active; + u16 flags, cltv_expiry_delta; + u32 timestamp, fee_base_msat, fee_proportional_millionths; + struct bitcoin_blkid chain_hash; + secp256k1_ecdsa_signature sig; + u64 htlc_minimum_msat; + + if (!fromwire_gossip_disable_channel(msg, NULL, &scid, &direction, &active) ) { + status_trace("Unable to parse %s", + gossip_wire_type_name(fromwire_peektype(msg))); + goto fail; + } + + nc = get_connection_by_scid(daemon->rstate, &scid, direction); + + if (!nc) { + status_trace( + "Unable to find channel %s/%d", + type_to_string(msg, struct short_channel_id, &scid), + direction); + goto fail; + } + + status_trace("Disabling channel %s/%d, active %d -> %d", + type_to_string(msg, struct short_channel_id, &scid), + direction, nc->active, active); + + nc->active = active; + + if (!nc->channel_update) { + status_trace( + "Channel %s/%d doesn't have a channel_update yet, can't " + "disable", + type_to_string(msg, struct short_channel_id, &scid), + direction); + goto fail; + } + + if (!fromwire_channel_update( + nc->channel_update, NULL, &sig, &chain_hash, &scid, ×tamp, + &flags, &cltv_expiry_delta, &htlc_minimum_msat, &fee_base_msat, + &fee_proportional_millionths)) { + status_failed( + STATUS_FAIL_INTERNAL_ERROR, + "Unable to parse previously accepted channel_update"); + } + + timestamp = time_now().ts.tv_sec; + if (timestamp <= nc->last_timestamp) + timestamp = nc->last_timestamp + 1; + + /* Active is bit 1 << 1, mask and apply */ + flags = (0xFFFD & flags) | (!active << 1); + + msg = towire_channel_update(tmpctx, &sig, &chain_hash, &scid, timestamp, + flags, cltv_expiry_delta, htlc_minimum_msat, + fee_base_msat, fee_proportional_millionths); + + if (!wire_sync_write(HSM_FD, + towire_hsm_cupdate_sig_req(tmpctx, msg))) { + status_failed(STATUS_FAIL_HSM_IO, "Writing cupdate_sig_req: %s", + strerror(errno)); + } + + msg = wire_sync_read(tmpctx, HSM_FD); + if (!msg || !fromwire_hsm_cupdate_sig_reply(tmpctx, msg, NULL, &msg)) { + status_failed(STATUS_FAIL_HSM_IO, + "Reading cupdate_sig_req: %s", + strerror(errno)); + } + + handle_channel_update(daemon->rstate, msg); + +fail: + tal_free(tmpctx); return daemon_conn_read_next(conn, &daemon->master); } From 5b86ddb6aa552611390143653487a193fd3b2c13 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 23 Jan 2018 22:15:29 +0100 Subject: [PATCH 6/9] masterd: Disable a channel when channeld fails Signed-off-by: Christian Decker --- lightningd/peer_control.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 3d3ac9c6df0d..6ba4cb2cb1e3 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -186,11 +186,17 @@ void peer_fail_permanent(struct peer *peer, const char *fmt, ...) { va_list ap; char *why; + u8 *msg; va_start(ap, fmt); why = tal_vfmt(peer, fmt, ap); va_end(ap); + if (peer->scid) { + msg = towire_gossip_disable_channel(peer, peer->scid, peer->direction, false); + subd_send_msg(peer->ld->gossip, take(msg)); + } + log_unusual(peer->log, "Peer permanent failure in %s: %s", peer_state_name(peer->state), why); From 7813e1fffe783c7b239b32bae34df9bc89ffb4ce Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 24 Jan 2018 14:18:12 +0100 Subject: [PATCH 7/9] pytest: Check that we disable on closing and permfail. Signed-off-by: Christian Decker --- tests/test_lightningd.py | 17 +++++++++++++++++ tests/utils.py | 5 ++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 56ce7c6ba447..3ce650840386 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -839,6 +839,15 @@ def test_closing(self): assert l1.bitcoin.rpc.getmempoolinfo()['size'] == 0 + l1.bitcoin.rpc.generate(5) + + # Only wait for the channels to activate with DEVELOPER=1, + # otherwise it's going to take too long because of the missing + # --dev-broadcast-interval + if DEVELOPER: + wait_for(lambda: len(l1.getactivechannels()) == 2) + wait_for(lambda: len(l2.getactivechannels()) == 2) + # This should return, then close. l1.rpc.close(l2.info['id']) l1.daemon.wait_for_log('-> CHANNELD_SHUTTING_DOWN') @@ -851,6 +860,10 @@ def test_closing(self): 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) + assert l1.bitcoin.rpc.getmempoolinfo()['size'] == 1 # Now grab the close transaction @@ -1166,6 +1179,9 @@ def test_penalty_inhtlc(self): # Now, this will get stuck due to l1 commit being disabled.. t = self.pay(l1,l2,100000000,async=True) + assert len(l1.getactivechannels()) == 1 + assert len(l2.getactivechannels()) == 1 + # They should both have commitments blocked now. l1.daemon.wait_for_log('=WIRE_COMMITMENT_SIGNED-nocommit') l2.daemon.wait_for_log('=WIRE_COMMITMENT_SIGNED-nocommit') @@ -1196,6 +1212,7 @@ def test_penalty_inhtlc(self): l2.daemon.wait_for_log('-> ONCHAIND_CHEATED') # FIXME: l1 should try to stumble along! + wait_for(lambda: len(l2.getactivechannels()) == 0) # l2 should spend all of the outputs (except to-us). # Could happen in any order, depending on commitment tx. diff --git a/tests/utils.py b/tests/utils.py index 5387fcbe0771..71d7f88bf194 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -90,7 +90,7 @@ def kill(self): self.proc.kill() self.proc.wait() self.thread.join() - + def tail(self): """Tail the stdout of the process and remember it. @@ -334,6 +334,9 @@ def openchannel(self, remote_node, capacity): self.bitcoin.generate_block(6) self.daemon.wait_for_log('-> CHANNELD_NORMAL|STATE_NORMAL') + def getactivechannels(self): + return [c for c in self.rpc.listchannels()['channels'] if c['active']] + def db_query(self, query): db = sqlite3.connect(os.path.join(self.daemon.lightning_dir, "lightningd.sqlite3")) db.row_factory = sqlite3.Row From 25bcaeb14700ded57930c7c36d7c8feba557fd9a Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 24 Jan 2018 15:10:26 +0100 Subject: [PATCH 8/9] pytest: Fix test_pay_disconnect We are now too quick in disabling the channel for us to attempt a payment. We need to separate into getroute and sendpay to trigger this now. Signed-off-by: Christian Decker --- tests/test_lightningd.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 3ce650840386..92331b6c158b 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -2960,20 +2960,23 @@ def test_pay_disconnect(self): # Wait for route propagation. self.wait_for_routes(l1, [chanid]) - inv = l2.rpc.invoice(123000, 'test_pay_disconnect', 'description')['bolt11'] + inv = l2.rpc.invoice(123000, 'test_pay_disconnect', 'description') + rhash = inv['payment_hash'] + + # Can't use `pay` since that'd notice that we can't route, due to disabling channel_update + route = l1.rpc.getroute(l2.info['id'], 123000, 1)["route"] # Make l2 upset by asking for crazy fee. l1.rpc.dev_setfees('150000') - # Wait for l1 notice l1.daemon.wait_for_log('STATUS_FAIL_PEER_BAD') # Can't pay while its offline. - self.assertRaises(ValueError, l1.rpc.pay, inv) + self.assertRaises(ValueError, l1.rpc.sendpay, to_json(route), rhash) l1.daemon.wait_for_log('Failing: first peer not ready: WIRE_TEMPORARY_CHANNEL_FAILURE') # Should fail due to temporary channel fail - self.assertRaises(ValueError, l1.rpc.pay, inv) + self.assertRaises(ValueError, l1.rpc.sendpay, to_json(route), rhash) l1.daemon.wait_for_log('Failing: first peer not ready: WIRE_TEMPORARY_CHANNEL_FAILURE') assert not l1.daemon.is_in_log('... still in progress') @@ -2981,9 +2984,5 @@ def test_pay_disconnect(self): bitcoind.generate_block(1) l1.daemon.wait_for_log('ONCHAIND_.*_UNILATERAL') - self.assertRaises(ValueError, l1.rpc.pay, inv) - # Could fail with almost anything, but should fail with WIRE_PERMANENT_CHANNEL_FAILURE or unknown route. FIXME - #l1.daemon.wait_for_log('Could not find a route') - if __name__ == '__main__': unittest.main(verbosity=2) From f4b14ba90ca90e4e83b30098f9cf62d92684fd48 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 25 Jan 2018 11:44:30 +0100 Subject: [PATCH 9/9] fixup! masterd: Disable a channel when channeld fails --- lightningd/peer_control.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 6ba4cb2cb1e3..742d81915bea 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -2097,7 +2098,7 @@ static bool peer_start_channeld(struct peer *peer, if (peer->ld->config.ignore_fee_limits) log_debug(peer->log, "Ignoring fee limits!"); - peer->direction = pubkey_cmp(&peer->ld->id, &peer->id) > 0; + peer->direction = get_channel_direction(&peer->ld->id, &peer->id); initmsg = towire_channel_init(tmpctx, &get_chainparams(peer->ld)