diff --git a/common/jsonrpc_errors.h b/common/jsonrpc_errors.h index 4fa4f37cfab7..35d5cab3de64 100644 --- a/common/jsonrpc_errors.h +++ b/common/jsonrpc_errors.h @@ -49,6 +49,8 @@ static const errcode_t FUNDING_BROADCAST_FAIL = 303; static const errcode_t FUNDING_STILL_SYNCING_BITCOIN = 304; static const errcode_t FUNDING_PEER_NOT_CONNECTED = 305; static const errcode_t FUNDING_UNKNOWN_PEER = 306; +static const errcode_t FUNDING_NOTHING_TO_CANCEL = 307; +static const errcode_t FUNDING_CANCEL_NOT_SAFE = 308; /* `connect` errors */ static const errcode_t CONNECT_NO_KNOWN_ADDRESS = 400; diff --git a/doc/lightning-fundchannel_cancel.7 b/doc/lightning-fundchannel_cancel.7 index f068a790fd08..a70c303dcee7 100644 --- a/doc/lightning-fundchannel_cancel.7 +++ b/doc/lightning-fundchannel_cancel.7 @@ -36,9 +36,13 @@ with \fBcode\fR being one of the following: .IP \[bu] -32602: If the given parameters are wrong\. .IP \[bu] --1: Catchall nonspecific error\. -.IP \[bu] 306: Unknown peer id\. +.IP \[bu] +307: No channel currently being funded that can be cancelled\. +.IP \[bu] +308: It is unsafe to cancel the channel: the funding transaction +has been broadcast, or there are HTLCs already in the channel, or +the peer was the initiator and not us\. .RE .SH AUTHOR diff --git a/doc/lightning-fundchannel_cancel.7.md b/doc/lightning-fundchannel_cancel.7.md index 068b247a18db..1960c4c8d5c0 100644 --- a/doc/lightning-fundchannel_cancel.7.md +++ b/doc/lightning-fundchannel_cancel.7.md @@ -32,8 +32,11 @@ On error the returned object will contain `code` and `message` properties, with `code` being one of the following: - -32602: If the given parameters are wrong. -- -1: Catchall nonspecific error. - 306: Unknown peer id. +- 307: No channel currently being funded that can be cancelled. +- 308: It is unsafe to cancel the channel: the funding transaction + has been broadcast, or there are HTLCs already in the channel, or + the peer was the initiator and not us. AUTHOR ------ diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 23a6ed0d5022..c6b7cade437a 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -752,7 +752,8 @@ static void process_check_funding_broadcast(struct bitcoind *bitcoind, if (txout != NULL) { for (size_t i = 0; i < tal_count(cancel->forgets); i++) - was_pending(command_fail(cancel->forgets[i], LIGHTNINGD, + was_pending(command_fail(cancel->forgets[i], + FUNDING_CANCEL_NOT_SAFE, "The funding transaction has been broadcast, " "please consider `close` or `dev-fail`! ")); tal_free(cancel->forgets); @@ -767,47 +768,41 @@ static void process_check_funding_broadcast(struct bitcoind *bitcoind, } struct command_result *cancel_channel_before_broadcast(struct command *cmd, - const char *buffer, - struct peer *peer, - const jsmntok_t *cidtok) + struct peer *peer) { struct channel *cancel_channel; struct channel_to_cancel *cc = tal(cmd, struct channel_to_cancel); + struct channel *channel; cc->peer = peer->id; - if (!cidtok) { - struct channel *channel; - - cancel_channel = NULL; - list_for_each(&peer->channels, channel, list) { - if (cancel_channel) { - return command_fail(cmd, LIGHTNINGD, - "Multiple channels:" - " please specify channel_id"); - } - cancel_channel = channel; - } - if (!cancel_channel) - return command_fail(cmd, LIGHTNINGD, - "No channels matching that peer_id"); - derive_channel_id(&cc->cid, - &cancel_channel->funding_txid, - cancel_channel->funding_outnum); - } else { - if (!json_tok_channel_id(buffer, cidtok, &cc->cid)) - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "Invalid channel_id parameter."); - - cancel_channel = find_channel_by_id(peer, &cc->cid); - if (!cancel_channel) - return command_fail(cmd, LIGHTNINGD, - "Channel ID not found: '%.*s'", - cidtok->end - cidtok->start, - buffer + cidtok->start); + cancel_channel = NULL; + list_for_each(&peer->channels, channel, list) { + /* After `fundchannel_complete`, channel is in + * `CHANNELD_AWAITING_LOCKIN` state. + * + * TODO: This assumes only one channel at a time + * can be in this state, which is true at the + * time of this writing, but may change *if* we + * ever implement multiple channels per peer. + */ + if (channel->state != CHANNELD_AWAITING_LOCKIN) + continue; + cancel_channel = channel; + break; } + if (!cancel_channel) + return command_fail(cmd, FUNDING_NOTHING_TO_CANCEL, + "No channels being opened or " + "awaiting lock-in for " + "peer_id %s", + type_to_string(tmpctx, struct node_id, + &peer->id)); + derive_channel_id(&cc->cid, + &cancel_channel->funding_txid, + cancel_channel->funding_outnum); if (cancel_channel->opener == REMOTE) - return command_fail(cmd, LIGHTNINGD, + return command_fail(cmd, FUNDING_CANCEL_NOT_SAFE, "Cannot cancel channel that was " "initiated by peer"); @@ -817,13 +812,13 @@ struct command_result *cancel_channel_before_broadcast(struct command *cmd, if (wallet_transaction_type(cmd->ld->wallet, &cancel_channel->funding_txid, &type)) - return command_fail(cmd, LIGHTNINGD, + return command_fail(cmd, FUNDING_CANCEL_NOT_SAFE, "Has the funding transaction been broadcast? " "Please use `close` or `dev-fail` instead."); if (channel_has_htlc_out(cancel_channel) || channel_has_htlc_in(cancel_channel)) { - return command_fail(cmd, LIGHTNINGD, + return command_fail(cmd, FUNDING_CANCEL_NOT_SAFE, "This channel has HTLCs attached and it is " "not safe to cancel. Has the funding transaction " "been broadcast? Please use `close` or `dev-fail` " diff --git a/lightningd/channel_control.h b/lightningd/channel_control.h index 10542e77fee2..cf234a5e773a 100644 --- a/lightningd/channel_control.h +++ b/lightningd/channel_control.h @@ -26,9 +26,7 @@ void channel_notify_new_block(struct lightningd *ld, /* Cancel the channel after `fundchannel_complete` succeeds * but before funding broadcasts. */ struct command_result *cancel_channel_before_broadcast(struct command *cmd, - const char *buffer, - struct peer *peer, - const jsmntok_t *cidtok); + struct peer *peer); /* Forget a channel. Deletes the channel and handles all * associated waiting commands, if present. Notifies peer if available */ diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index fb4a01c15734..5343c384ee13 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -308,29 +308,7 @@ static void cancel_after_fundchannel_complete_success(struct command *cmd, struct channel *channel) { - struct peer *peer; - struct channel_id cid; - /* Fake these to adapt to the existing - * cancel_channel_before_broadcast - * interface. - */ - const char *buffer; - jsmntok_t cidtok; - - peer = channel->peer; - derive_channel_id(&cid, - &channel->funding_txid, channel->funding_outnum); - - buffer = type_to_string(tmpctx, struct channel_id, &cid); - cidtok.type = JSMN_STRING; - cidtok.start = 0; - cidtok.end = strlen(buffer); - cidtok.size = 0; - - was_pending(cancel_channel_before_broadcast(cmd, - buffer, - peer, - &cidtok)); + was_pending(cancel_channel_before_broadcast(cmd, channel->peer)); } static void funding_success(struct channel *channel) @@ -1150,12 +1128,10 @@ static struct command_result *json_fund_channel_cancel(struct command *cmd, struct node_id *id; struct peer *peer; - const jsmntok_t *cidtok; u8 *msg; if (!param(cmd, buffer, params, p_req("id", param_node_id, &id), - p_opt("channel_id", param_tok, &cidtok), NULL)) return command_param_failed(); @@ -1166,7 +1142,8 @@ static struct command_result *json_fund_channel_cancel(struct command *cmd, if (peer->uncommitted_channel) { if (!peer->uncommitted_channel->fc || !peer->uncommitted_channel->fc->inflight) - return command_fail(cmd, LIGHTNINGD, "No channel funding in progress."); + return command_fail(cmd, FUNDING_NOTHING_TO_CANCEL, + "No channel funding in progress."); /* Make sure this gets notified if we succeed or cancel */ tal_arr_expand(&peer->uncommitted_channel->fc->cancels, cmd); @@ -1175,7 +1152,8 @@ static struct command_result *json_fund_channel_cancel(struct command *cmd, return command_still_pending(cmd); } - return cancel_channel_before_broadcast(cmd, buffer, peer, cidtok); + /* Handle `fundchannel_cancel` after `fundchannel_complete`. */ + return cancel_channel_before_broadcast(cmd, peer); } /** diff --git a/tests/test_connection.py b/tests/test_connection.py index f7e1ad0637ce..deeabec3af8d 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1163,8 +1163,8 @@ def _fundchannel(l1, l2, amount, close_to): @unittest.skipIf(TEST_NETWORK != 'regtest', "External wallet support doesn't work with elements yet.") def test_funding_external_wallet(node_factory, bitcoind): - l1 = node_factory.get_node() - l2 = node_factory.get_node() + l1 = node_factory.get_node(options={'funding-confirms': 2}) + l2 = node_factory.get_node(options={'funding-confirms': 2}) l3 = node_factory.get_node() l1.rpc.connect(l2.info['id'], 'localhost', l2.port) @@ -1198,20 +1198,23 @@ def test_funding_external_wallet(node_factory, bitcoind): assert l1.rpc.fundchannel_complete(l2.info['id'], txid, txout)['commitments_secured'] - # Broadcast the transaction manually and confirm that channel locks in + # Broadcast the transaction manually signed_tx = bitcoind.rpc.signrawtransactionwithwallet(raw_funded_tx)['hex'] assert txid == bitcoind.rpc.decoderawtransaction(signed_tx)['txid'] bitcoind.rpc.sendrawtransaction(signed_tx) bitcoind.generate_block(1) - l1.daemon.wait_for_log(r'Funding tx {} depth 1 of 1'.format(txid)) + l1.daemon.wait_for_log(r'Funding tx {} depth 1 of 2'.format(txid)) # Check that tx is broadcast by a third party can be catched. # Only when the transaction (broadcast by a third pary) is onchain, we can catch it. with pytest.raises(RpcError, match=r'.* been broadcast.*'): l1.rpc.fundchannel_cancel(l2.info['id']) + # Confirm that channel locks in + bitcoind.generate_block(1) + for node in [l1, l2]: node.daemon.wait_for_log(r'State changed from CHANNELD_AWAITING_LOCKIN to CHANNELD_NORMAL') channel = node.rpc.listpeers()['peers'][0]['channels'][0]