From 909cd4136b6a6e77bed84fcc8f31bc1329d3741d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 9 Aug 2018 09:55:29 +0930 Subject: [PATCH] openingd: get told if we can't let them open a new channel. Previously master would fail once the channel has been negotiated, which is terrible, since the funder will have already broadcast tx. Now we tell them if we have an active channel, and update if it goes away. Signed-off-by: Rusty Russell --- lightningd/channel.c | 8 ++++++++ lightningd/opening_control.c | 30 +++++++++++++----------------- lightningd/opening_control.h | 2 ++ openingd/opening.c | 19 +++++++++++++++++++ openingd/opening_wire.csv | 4 ++++ wallet/test/run-wallet.c | 3 +++ 6 files changed, 49 insertions(+), 17 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 4d9668dfe..a0a41b644 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -300,6 +301,8 @@ void channel_set_state(struct channel *channel, enum channel_state old_state, enum channel_state state) { + bool was_active = channel_active(channel); + log_info(channel->log, "State changed from %s to %s", channel_state_name(channel), channel_state_str(state)); if (channel->state != old_state) @@ -310,6 +313,11 @@ void channel_set_state(struct channel *channel, /* TODO(cdecker) Selectively save updated fields to DB */ wallet_channel_save(channel->peer->ld->wallet, channel); + + /* If openingd is running, it might want to know we're no longer + * active */ + if (was_active && !channel_active(channel)) + opening_peer_no_active_channels(channel->peer); } void channel_fail_permanent(struct channel *channel, const char *fmt, ...) diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index ca337cef1..e45b19e01 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -463,26 +463,11 @@ static void opening_fundee_finished(struct subd *openingd, return; } - /* If we resumed chatting with them to send an error, this could - * happen: refuse to let them open another active channel. - * - * FIXME: Perhaps we should not consider channels with errors to be - * active, however we don't store errors in the db so we could end - * up with multiple on restart. */ + /* openingd should never accept them funding channel in this case. */ if (peer_active_channel(uc->peer)) { - u8 *errmsg; - struct peer *peer = uc->peer; - struct channel_id channel_id; - - derive_channel_id(&channel_id, &funding_txid, funding_outnum); - errmsg = towire_errorfmt(tmpctx, &channel_id, - "Already have active channel"); - + log_broken(uc->log, "openingd accepted peer funding channel"); /* Won't free peer, since has active channel */ tal_free(uc); - - /* Hand back to openingd. */ - peer_start_openingd(peer, &cs, fds[0], fds[1], errmsg); return; } @@ -663,6 +648,7 @@ static unsigned int openingd_msg(struct subd *openingd, /* We send these! */ case WIRE_OPENING_INIT: case WIRE_OPENING_FUNDER: + case WIRE_OPENING_CAN_ACCEPT_CHANNEL: break; } log_broken(openingd->log, "Unexpected msg %s: %s", @@ -727,10 +713,20 @@ void peer_start_openingd(struct peer *peer, &uc->local_funding_pubkey, uc->minimum_depth, feerate_min(peer->ld), feerate_max(peer->ld), + !peer_active_channel(peer), send_msg); subd_send_msg(uc->openingd, take(msg)); } +void opening_peer_no_active_channels(struct peer *peer) +{ + assert(!peer_active_channel(peer)); + if (peer->uncommitted_channel) { + subd_send_msg(peer->uncommitted_channel->openingd, + take(towire_opening_can_accept_channel(NULL))); + } +} + /** * json_fund_channel - Entrypoint for funding a channel */ diff --git a/lightningd/opening_control.h b/lightningd/opening_control.h index cc22a9c58..59bb059ad 100644 --- a/lightningd/opening_control.h +++ b/lightningd/opening_control.h @@ -17,6 +17,8 @@ void peer_start_openingd(struct peer *peer, int peer_fd, int gossip_fd, const u8 *msg); +void opening_peer_no_active_channels(struct peer *peer); + void kill_uncommitted_channel(struct uncommitted_channel *uc, const char *why); diff --git a/openingd/opening.c b/openingd/opening.c index a893ac652..50321900a 100644 --- a/openingd/opening.c +++ b/openingd/opening.c @@ -69,6 +69,7 @@ struct state { struct channel *channel; + bool can_accept_channel; const struct chainparams *chainparams; }; @@ -601,6 +602,17 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) "Bad open_channel %s", tal_hex(open_channel_msg, open_channel_msg)); + /* We can't handle talking about more than one channel at once. */ + if (!state->can_accept_channel) { + u8 *errmsg; + errmsg = towire_errorfmt(NULL, &state->channel_id, + "Already have active channel"); + + sync_crypto_write(&state->cs, PEER_FD, take(errmsg)); + state->remoteconf = tal_free(state->remoteconf); + return NULL; + } + /* BOLT #2: * * The receiver: @@ -951,6 +963,12 @@ static u8 *handle_master_in(struct state *state) "Funding channel: opening negotiation succeeded"); return msg; + case WIRE_OPENING_CAN_ACCEPT_CHANNEL: + if (!fromwire_opening_can_accept_channel(msg)) + master_badmsg(WIRE_OPENING_CAN_ACCEPT_CHANNEL, msg); + state->can_accept_channel = true; + return NULL; + case WIRE_OPENING_INIT: case WIRE_OPENING_FUNDER_REPLY: case WIRE_OPENING_FUNDEE: @@ -986,6 +1004,7 @@ int main(int argc, char *argv[]) &state->our_funding_pubkey, &state->minimum_depth, &state->min_feerate, &state->max_feerate, + &state->can_accept_channel, &inner)) master_badmsg(WIRE_OPENING_INIT, msg); diff --git a/openingd/opening_wire.csv b/openingd/opening_wire.csv index 9091288a8..13bbbba40 100644 --- a/openingd/opening_wire.csv +++ b/openingd/opening_wire.csv @@ -17,10 +17,14 @@ opening_init,,our_funding_pubkey,struct pubkey opening_init,,minimum_depth,u32 opening_init,,min_feerate,u32 opening_init,,max_feerate,u32 +opening_init,,can_open_channel,bool # Optional msg to send. opening_init,,len,u16 opening_init,,msg,len*u8 +# Master->openingd: they can now open a channel if they want. +opening_can_accept_channel,6002 + #include #include # Master->openingd: please fund a channel. diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 7e95c35cf..09f8de9b1 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -256,6 +256,9 @@ enum watch_result onchaind_funding_spent(struct channel *channel UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, u32 blockheight UNNEEDED) { fprintf(stderr, "onchaind_funding_spent called!\n"); abort(); } +/* Generated stub for opening_peer_no_active_channels */ +void opening_peer_no_active_channels(struct peer *peer UNNEEDED) +{ fprintf(stderr, "opening_peer_no_active_channels called!\n"); abort(); } /* Generated stub for outpointfilter_add */ void outpointfilter_add(struct outpointfilter *of UNNEEDED, const struct bitcoin_txid *txid UNNEEDED, const u32 outnum UNNEEDED)