Skip to content
11 changes: 10 additions & 1 deletion channeld/channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -1522,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,
Expand Down
88 changes: 88 additions & 0 deletions gossipd/gossip.c
Original file line number Diff line number Diff line change
Expand Up @@ -1769,6 +1769,91 @@ 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)
{
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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, what if active was true? Did we just enable a channel w/o a channel update?

I think this test should be moved up above the assignment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't do that, otherwise we can only ever deactivate a channel that has reached announce_depth, i.e., 6 blocks. The channel is activated locally after funding_depth, i.e., 2 confirmations. So if we happen to lose the connection or similar, which the channel has 2-5 confirmations we cannot deactivate and we're stuck with a non-functioning channel being returned as part of the routes.

If the channel is public, we also have a channel_update which we can update with the disable flag, if the channel is not public, then others don't care anyway.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

OK, my q still stands. We're asserting that active == false in this branch (based on the msg), we should actually assert that?

I'll apply this now, but please consider...

}

if (!fromwire_channel_update(
nc->channel_update, NULL, &sig, &chain_hash, &scid, &timestamp,
&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);
}

static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master)
{
struct daemon *daemon = container_of(master, struct daemon, master);
Expand Down Expand Up @@ -1814,6 +1899,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:
Expand Down
5 changes: 5 additions & 0 deletions gossipd/gossip_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions lightningd/gossip_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 9 additions & 0 deletions lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <errno.h>
#include <fcntl.h>
#include <gossipd/gen_gossip_wire.h>
#include <gossipd/routing.h>
#include <hsmd/capabilities.h>
#include <hsmd/gen_hsm_client_wire.h>
#include <inttypes.h>
Expand Down Expand Up @@ -186,11 +187,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);

Expand Down Expand Up @@ -2091,6 +2098,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 = get_channel_direction(&peer->ld->id, &peer->id);

initmsg = towire_channel_init(tmpctx,
&get_chainparams(peer->ld)
->genesis_blockhash,
Expand Down
3 changes: 3 additions & 0 deletions lightningd/peer_control.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
32 changes: 24 additions & 8 deletions tests/test_lightningd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -2943,30 +2960,29 @@ 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')

# After it sees block, someone should close channel.
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)
5 changes: 4 additions & 1 deletion tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand Down