diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index 24e0f7d60..810d4a45e 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -361,6 +361,7 @@ static void openchannel2_hook_cb(struct openchannel2_payload *payload STEALS) { struct subd *dualopend = payload->dualopend; + struct uncommitted_channel *uc; u8 *msg; /* Free payload regardless of what happens next */ @@ -370,6 +371,18 @@ openchannel2_hook_cb(struct openchannel2_payload *payload STEALS) if (!dualopend) return; + assert(dualopend->ctype == UNCOMMITTED); + uc = dualopend->channel; + + /* Channel open is currently in progress elsewhere! */ + if (uc->fc || uc->got_offer) { + msg = towire_dualopend_fail(NULL, "Already initiated channel" + " open"); + log_debug(dualopend->ld->log, + "Our open in progress, denying their offer"); + return subd_send_msg(dualopend, take(msg)); + } + tal_del_destructor2(dualopend, openchannel2_remove_dualopend, payload); if (payload->err_msg) { @@ -407,6 +420,7 @@ openchannel2_hook_cb(struct openchannel2_payload *payload STEALS) } } + uc->got_offer = true; msg = towire_dualopend_got_offer_reply(NULL, payload->accepter_funding, payload->funding_feerate_per_kw, payload->psbt, @@ -1146,6 +1160,7 @@ static void open_failed(struct subd *dualopend, const u8 *msg) assert(dualopend->ctype == UNCOMMITTED); uc = dualopend->channel; + uc->got_offer = false; if (!fromwire_dualopend_failed(msg, msg, &desc)) { log_broken(uc->log, @@ -1427,7 +1442,15 @@ static void accepter_got_offer(struct subd *dualopend, if (peer_active_channel(uc->peer)) { subd_send_msg(dualopend, - take(towire_dualopend_fail(NULL, "Already have active channel"))); + take(towire_dualopend_fail(NULL, + "Already have active channel"))); + return; + } + + if (uc->fc) { + subd_send_msg(dualopend, + take(towire_dualopend_fail(NULL, + "Already initiated channel open"))); return; } @@ -1773,6 +1796,11 @@ static struct command_result *json_openchannel_init(struct command *cmd, return command_fail(cmd, LIGHTNINGD, "Already funding channel"); } + if (peer->uncommitted_channel->got_offer) { + return command_fail(cmd, LIGHTNINGD, + "Channel open in progress"); + } + #if EXPERIMENTAL_FEATURES if (!feature_negotiated(cmd->ld->our_features, peer->their_features, diff --git a/openingd/dualopend.c b/openingd/dualopend.c index 21cc049e5..c5cc84971 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -277,16 +277,13 @@ static void negotiation_aborted(struct state *state, const char *why) memset(&state->channel_id, 0, sizeof(state->channel_id)); state->channel = tal_free(state->channel); state->changeset = tal_free(state->changeset); - if (state->psbt) - tal_free(state->psbt); for (size_t i = 0; i < NUM_TX_MSGS; i++) state->tx_msg_count[i] = 0; } -/*~ For negotiation failures: we tell them the parameter we didn't like. */ -static void negotiation_failed(struct state *state, - const char *fmt, ...) +static void open_error(struct state *state, + const char *fmt, ...) { va_list ap; const char *errmsg; @@ -297,12 +294,27 @@ static void negotiation_failed(struct state *state, va_end(ap); msg = towire_errorfmt(NULL, &state->channel_id, - "You gave bad parameters: %s", errmsg); + "%s", errmsg); sync_crypto_write(state->pps, take(msg)); negotiation_aborted(state, errmsg); } + +/*~ For negotiation failures: we tell them the parameter we didn't like. */ +static void negotiation_failed(struct state *state, + const char *fmt, ...) +{ + va_list ap; + const char *errmsg; + + va_start(ap, fmt); + errmsg = tal_vfmt(tmpctx, fmt, ap); + va_end(ap); + + open_error(state, "You gave bad parameters: %s", errmsg); +} + static void billboard_update(struct state *state) { const char *update = billboard_message(tmpctx, state->funding_locked, @@ -885,9 +897,9 @@ fetch_psbt_changes(struct state *state, const struct wally_psbt *psbt) wire_sync_write(REQ_FD, take(msg)); msg = wire_sync_read(tmpctx, REQ_FD); - if (fromwire_dualopend_fail(msg, msg, &err)) - status_failed(STATUS_FAIL_MASTER_IO, "%s", err); - else if (fromwire_dualopend_psbt_updated(state, msg, &updated_psbt)) { + if (fromwire_dualopend_fail(msg, msg, &err)) { + open_error(state, "%s", err); + } else if (fromwire_dualopend_psbt_updated(state, msg, &updated_psbt)) { return updated_psbt; #if DEVELOPER } else if (fromwire_dualopend_dev_memleak(msg)) { @@ -1507,7 +1519,7 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) if (!fromwire_dualopend_fail(msg, msg, &err_reason)) master_badmsg(msg_type, msg); - negotiation_failed(state, "%s", err_reason); + open_error(state, "%s", err_reason); return; } @@ -1927,7 +1939,7 @@ static void opener_start(struct state *state, u8 *msg) if (!msg) return; - a_tlv = tlv_accept_tlvs_new(state); + a_tlv = notleak(tlv_accept_tlvs_new(state)); if (!fromwire_accept_channel2(msg, &cid, &state->accepter_funding, &state->feerate_per_kw_funding, @@ -2017,9 +2029,11 @@ static void opener_start(struct state *state, u8 *msg) * - MUST send at least one `tx_add_output`, the channel * funding output. */ - wscript = bitcoin_redeem_2of2(state, - &state->our_funding_pubkey, - &state->their_funding_pubkey); + /* If fails before returning from `send_next`, this will + * be marked as a memleak */ + wscript = notleak(bitcoin_redeem_2of2(state, + &state->our_funding_pubkey, + &state->their_funding_pubkey)); funding_out = psbt_append_output(state->psbt, scriptpubkey_p2wsh(tmpctx, wscript), @@ -2052,7 +2066,7 @@ static void opener_start(struct state *state, u8 *msg) /* Send our first message, we're opener we initiate here */ if (!send_next(state, &state->psbt)) - negotiation_failed(state, "Peer error, no updates to send"); + open_error(state, "Peer error, no updates to send"); /* Figure out what the funding transaction looks like! */ if (!run_tx_interactive(state, &state->psbt, TX_INITIATOR)) diff --git a/tests/test_connection.py b/tests/test_connection.py index 572c71798..91fd0636f 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -3026,3 +3026,24 @@ def test_fundchannel_start_alternate(node_factory, executor): fut = executor.submit(l2.rpc.fundchannel_start, l1.info['id'], 100000) with pytest.raises(RpcError): fut.result(10) + + +@unittest.skipIf(not EXPERIMENTAL_DUAL_FUND, "openchannel_init not available") +def test_openchannel_init_alternate(node_factory, executor): + ''' Test to see what happens if two nodes start channeling to + each other alternately. + ''' + l1, l2 = node_factory.get_nodes(2) + + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + + l1.fundwallet(2000000) + l2.fundwallet(2000000) + + psbt1 = l1.rpc.fundpsbt('1000000msat', '253perkw', 250)['psbt'] + psbt2 = l2.rpc.fundpsbt('1000000msat', '253perkw', 250)['psbt'] + l1.rpc.openchannel_init(l2.info['id'], 100000, psbt1) + + fut = executor.submit(l2.rpc.openchannel_init, l1.info['id'], '1000000msat', psbt2) + with pytest.raises(RpcError): + fut.result(10)