From b62706aa01f1530f6eb21ab0a337b840b042ecda Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 16 Mar 2021 06:56:12 +1030 Subject: [PATCH] close: accept wrong_funding outpoint arg if we negotiated the feature. Changelog-Added: lightningd: experimental-shutdown-wrong-funding to allow remote nodes to close incorrectly opened channels. Changelog-Added: JSON-RPC: close has a new `wrong_funding` option to try to close out unused channels where we messed up the funding tx. Signed-off-by: Rusty Russell --- doc/lightning-close.7 | 16 +++++-- doc/lightning-close.7.md | 13 +++++- doc/lightningd-config.5 | 8 ++-- doc/lightningd-config.5.md | 6 +-- lightningd/peer_control.c | 50 ++++++++++++++++++-- lightningd/test/run-invoice-select-inchan.c | 7 +++ tests/test_closing.py | 51 ++++++++++++++++++++- wallet/db_postgres_sqlgen.c | 2 +- wallet/db_sqlite3_sqlgen.c | 2 +- wallet/statements_gettextgen.po | 6 +-- wallet/test/run-wallet.c | 7 +++ 11 files changed, 146 insertions(+), 22 deletions(-) diff --git a/doc/lightning-close.7 b/doc/lightning-close.7 index 405441ad1..0bee4ae3c 100644 --- a/doc/lightning-close.7 +++ b/doc/lightning-close.7 @@ -3,7 +3,7 @@ lightning-close - Command for closing channels with direct peers .SH SYNOPSIS -\fBclose\fR \fIid\fR [\fIunilateraltimeout\fR] [\fIdestination\fR] [\fIfee_negotiation_step\fR] +\fBclose\fR \fIid\fR [\fIunilateraltimeout\fR] [\fIdestination\fR] [\fIfee_negotiation_step\fR] [\fIwrong_funding\\\fR] .SH DESCRIPTION @@ -54,6 +54,16 @@ The default is "50%"\. .RE +\fIwrong_funding_txid\fR can only be specified if both sides have offered +the "shutdown_wrong_funding" feature (enabled by the +\fBexperimental-shutdown-wrong-funding\fR option): it must be a +transaction id followed by a colon then the output number\. Instead of +negotiating a shutdown to spend the expected funding transaction, the +shutdown transaction will spend this output instead\. This is only +allowed if this peer opened the channel and the channel is unused: it +can rescue openings which have been manually miscreated\. + + The peer needs to be live and connected in order to negotiate a mutual close\. The default of unilaterally closing after 48 hours is usually a reasonable indication that you can no longer contact the peer\. @@ -95,10 +105,10 @@ ZmnSCPxj \fI is mainly responsible\. .SH SEE ALSO -\fBlightning-disconnect\fR(7), \fBlightning-fundchannel\fR(7) +\fBlightning-disconnect\fR(7), \fBlightning-fundchannel\fR(7), \fBlightningd-config\fR(5)\. .SH RESOURCES Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -\" SHA256STAMP:d16e185f9a781f23987dfb65aaa1eae8dab19796975433c69e16f1f6b18751c5 +\" SHA256STAMP:f835c6a11e0df5610761e858e78783f9f99d530b8d2a6bedaf6f301eccdc31e0 diff --git a/doc/lightning-close.7.md b/doc/lightning-close.7.md index 411493cc1..d33654498 100644 --- a/doc/lightning-close.7.md +++ b/doc/lightning-close.7.md @@ -4,7 +4,7 @@ lightning-close -- Command for closing channels with direct peers SYNOPSIS -------- -**close** *id* \[*unilateraltimeout*\] \[*destination*\] \[*fee_negotiation_step*\] +**close** *id* \[*unilateraltimeout*\] \[*destination*\] \[*fee_negotiation_step*\] \[*wrong_funding\*] DESCRIPTION ----------- @@ -44,6 +44,15 @@ insist on our fee as much as possible. we quickly accept the peer's proposal. The default is "50%". +*wrong_funding_txid* can only be specified if both sides have offered +the "shutdown_wrong_funding" feature (enabled by the +**experimental-shutdown-wrong-funding** option): it must be a +transaction id followed by a colon then the output number. Instead of +negotiating a shutdown to spend the expected funding transaction, the +shutdown transaction will spend this output instead. This is only +allowed if this peer opened the channel and the channel is unused: it +can rescue openings which have been manually miscreated. + The peer needs to be live and connected in order to negotiate a mutual close. The default of unilaterally closing after 48 hours is usually a reasonable indication that you can no longer contact the peer. @@ -87,7 +96,7 @@ ZmnSCPxj <> is mainly responsible. SEE ALSO -------- -lightning-disconnect(7), lightning-fundchannel(7) +lightning-disconnect(7), lightning-fundchannel(7), lightningd-config(5). RESOURCES --------- diff --git a/doc/lightningd-config.5 b/doc/lightningd-config.5 index 887b848d6..9e7693c82 100644 --- a/doc/lightningd-config.5 +++ b/doc/lightningd-config.5 @@ -588,9 +588,9 @@ This usually requires \fBexperimental-onion-messages\fR as well\. See Specifying this allows the \fBwrong_funding\fR field in shutdown: if a -remote node has opened a channel using the incorrect txid (and it -hasn't been used yet at all) this allows them to negotiate a clean -shutdown with the txid they offer\. +remote node has opened a channel but claims it used the incorrect txid +(and the channel hasn't been used yet at all) this allows them to +negotiate a clean shutdown with the txid they offer\. .SH BUGS @@ -617,4 +617,4 @@ Main web site: \fIhttps://github.com/ElementsProject/lightning\fR Note: the modules in the ccan/ directory have their own licenses, but the rest of the code is covered by the BSD-style MIT license\. -\" SHA256STAMP:ecfd7ac60b50a20cad39dfa0e423e4885256a94658b5886f577df50773555e7e +\" SHA256STAMP:b0cdc467650b05758f90f2523ab8b45e526ac818e426cb2c2bc42cb98673e373 diff --git a/doc/lightningd-config.5.md b/doc/lightningd-config.5.md index 67ae212e0..a441730e7 100644 --- a/doc/lightningd-config.5.md +++ b/doc/lightningd-config.5.md @@ -486,9 +486,9 @@ lightning-offer(7) and lightning-fetchinvoice(7). **experimental-shutdown-wrong-funding** Specifying this allows the `wrong_funding` field in shutdown: if a -remote node has opened a channel using the incorrect txid (and it -hasn't been used yet at all) this allows them to negotiate a clean -shutdown with the txid they offer. +remote node has opened a channel but claims it used the incorrect txid +(and the channel hasn't been used yet at all) this allows them to +negotiate a clean shutdown with the txid they offer. BUGS ---- diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index f504609dd..c3e6a85d6 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1492,6 +1492,19 @@ command_find_channel(struct command *cmd, } } +static struct command_result *param_outpoint(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + struct bitcoin_outpoint **outp) +{ + *outp = tal(cmd, struct bitcoin_outpoint); + if (json_to_outpoint(buffer, tok, *outp)) + return NULL; + return command_fail_badparam(cmd, name, buffer, tok, + "should be a txid:outnum"); +} + static struct command_result *json_close(struct command *cmd, const char *buffer, const jsmntok_t *obj UNNEEDED, @@ -1502,8 +1515,9 @@ static struct command_result *json_close(struct command *cmd, struct channel *channel COMPILER_WANTS_INIT("gcc 7.3.0 fails, 8.3 OK"); unsigned int *timeout; const u8 *close_to_script = NULL; - bool close_script_set; + bool close_script_set, wrong_funding_changed; const char *fee_negotiation_step_str; + struct bitcoin_outpoint *wrong_funding; char* end; if (!param(cmd, buffer, params, @@ -1513,6 +1527,7 @@ static struct command_result *json_close(struct command *cmd, p_opt("destination", param_bitcoin_address, &close_to_script), p_opt("fee_negotiation_step", param_string, &fee_negotiation_step_str), + p_opt("wrong_funding", param_outpoint, &wrong_funding), NULL)) return command_param_failed(); @@ -1618,6 +1633,34 @@ static struct command_result *json_close(struct command *cmd, fee_negotiation_step_str); } + if (wrong_funding) { + if (!feature_negotiated(cmd->ld->our_features, + channel->peer->their_features, + OPT_SHUTDOWN_WRONG_FUNDING)) { + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "wrong_funding feature not negotiated" + " (we said %s, they said %s: try experimental-shutdown-wrong-funding?)", + feature_offered(cmd->ld->our_features + ->bits[INIT_FEATURE], + OPT_SHUTDOWN_WRONG_FUNDING) + ? "yes" : "no", + feature_offered(channel->peer->their_features, + OPT_SHUTDOWN_WRONG_FUNDING) + ? "yes" : "no"); + } + + wrong_funding_changed = true; + channel->shutdown_wrong_funding + = tal_steal(channel, wrong_funding); + } else { + if (channel->shutdown_wrong_funding) { + channel->shutdown_wrong_funding + = tal_free(channel->shutdown_wrong_funding); + wrong_funding_changed = true; + } else + wrong_funding_changed = false; + } + /* Normal case. * We allow states shutting down and sigexchange; a previous * close command may have timed out, and this current command @@ -1663,8 +1706,9 @@ static struct command_result *json_close(struct command *cmd, /* Register this command for later handling. */ register_close_command(cmd->ld, cmd, channel, *timeout); - /* If we set `channel->shutdown_scriptpubkey[LOCAL]`, save it. */ - if (close_script_set) + /* If we set `channel->shutdown_scriptpubkey[LOCAL]` or + * changed shutdown_wrong_funding, save it. */ + if (close_script_set || wrong_funding_changed) wallet_channel_save(cmd->ld->wallet, channel); /* Wait until close drops down to chain. */ diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 681cdf762..c3ab8f5f9 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -172,6 +172,9 @@ bool feature_is_set(const u8 *features UNNEEDED, size_t bit UNNEEDED) bool feature_negotiated(const struct feature_set *our_features UNNEEDED, const u8 *their_features UNNEEDED, size_t f UNNEEDED) { fprintf(stderr, "feature_negotiated called!\n"); abort(); } +/* Generated stub for feature_offered */ +bool feature_offered(const u8 *features UNNEEDED, size_t f UNNEEDED) +{ fprintf(stderr, "feature_offered called!\n"); abort(); } /* Generated stub for fixup_htlcs_out */ void fixup_htlcs_out(struct lightningd *ld UNNEEDED) { fprintf(stderr, "fixup_htlcs_out called!\n"); abort(); } @@ -374,6 +377,10 @@ enum address_parse_result json_to_address_scriptpubkey(const tal_t *ctx UNNEEDED bool json_to_node_id(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct node_id *id UNNEEDED) { fprintf(stderr, "json_to_node_id called!\n"); abort(); } +/* Generated stub for json_to_outpoint */ +bool json_to_outpoint(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, + struct bitcoin_outpoint *op UNNEEDED) +{ fprintf(stderr, "json_to_outpoint called!\n"); abort(); } /* Generated stub for json_to_short_channel_id */ bool json_to_short_channel_id(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct short_channel_id *scid UNNEEDED) diff --git a/tests/test_closing.py b/tests/test_closing.py index 0a1c6130c..b0699ec54 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -1,12 +1,12 @@ from fixtures import * # noqa: F401,F403 from flaky import flaky -from pyln.client import RpcError +from pyln.client import RpcError, Millisatoshi from shutil import copyfile from pyln.testing.utils import SLOW_MACHINE from utils import ( only_one, sync_blockheight, wait_for, DEVELOPER, TIMEOUT, account_balance, first_channel_id, basic_fee, TEST_NETWORK, - EXPERIMENTAL_FEATURES, + EXPERIMENTAL_FEATURES, EXPERIMENTAL_DUAL_FUND, ) import os @@ -2695,3 +2695,50 @@ Try a range of future segwit versions as shutdown scripts. We create many nodes l1.rpc.connect(l2.info['id'], 'localhost', l2.port) with pytest.raises(RpcError, match=r'Unacceptable upfront_shutdown_script'): l1.rpc.fundchannel(l2.info['id'], 10**6) + + +@unittest.skipIf(EXPERIMENTAL_DUAL_FUND, "Uses fundchannel_start") +def test_shutdown_alternate_txid(node_factory, bitcoind): + l1, l2 = node_factory.line_graph(2, fundchannel=False, + opts={'experimental-shutdown-wrong-funding': None}) + + amount = 1000000 + amount_msat = Millisatoshi(amount * 1000) + + # Let's make a classic fundchannel mistake (wrong txid!) + addr = l1.rpc.fundchannel_start(l2.info['id'], amount_msat)['funding_address'] + txid = bitcoind.rpc.sendtoaddress(addr, amount / 10**8) + + # Gotta figure out which output manually :( + tx = bitcoind.rpc.getrawtransaction(txid, 1) + for n, out in enumerate(tx['vout']): + if 'addresses' in out['scriptPubKey'] and out['scriptPubKey']['addresses'][0] == addr: + txout = n + + bitcoind.generate_block(1, wait_for_mempool=1) + + # Wrong txid, wrong txout! + wrong_txid = txid[16:] + txid[:16] + wrong_txout = txout ^ 1 + l1.rpc.fundchannel_complete(l2.info['id'], wrong_txid, wrong_txout) + + wait_for(lambda: only_one(l2.rpc.listpeers()['peers'])['channels'] != []) + wait_for(lambda: only_one(l2.rpc.listpeers()['peers'])['channels'][0]['state'] == 'CHANNELD_AWAITING_LOCKIN') + + closeaddr = l1.rpc.newaddr()['bech32'] + + # Oops, try rescuing it! + l1.rpc.call('close', {'id': l2.info['id'], 'destination': closeaddr, 'wrong_funding': txid + ':' + str(txout)}) + + # Just make sure node has no funds. + assert l1.rpc.listfunds()['outputs'] == [] + + bitcoind.generate_block(100, wait_for_mempool=1) + sync_blockheight(bitcoind, [l1, l2]) + + # We will see our funds return. + assert len(l1.rpc.listfunds()['outputs']) == 1 + + # FIXME: we should close channels, but we don't! + # wait_for(lambda: l2.rpc.listpeers()['peers'] == []) + # wait_for(lambda: l1.rpc.listpeers()['peers'] == []) diff --git a/wallet/db_postgres_sqlgen.c b/wallet/db_postgres_sqlgen.c index c115c3adf..35845f36f 100644 --- a/wallet/db_postgres_sqlgen.c +++ b/wallet/db_postgres_sqlgen.c @@ -1888,4 +1888,4 @@ struct db_query db_postgres_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_POSTGRES */ -// SHA256STAMP:2456747feb02494cb6237921d03c41c45fc86cf20289f4b5c379db7e548761f7 +// SHA256STAMP:fa582dba4c41760ea1760e8c98a53ca4a450ab8236d68bc8749eda0efb8c59af diff --git a/wallet/db_sqlite3_sqlgen.c b/wallet/db_sqlite3_sqlgen.c index 388a3462c..d486f6316 100644 --- a/wallet/db_sqlite3_sqlgen.c +++ b/wallet/db_sqlite3_sqlgen.c @@ -1888,4 +1888,4 @@ struct db_query db_sqlite3_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_SQLITE3 */ -// SHA256STAMP:2456747feb02494cb6237921d03c41c45fc86cf20289f4b5c379db7e548761f7 +// SHA256STAMP:fa582dba4c41760ea1760e8c98a53ca4a450ab8236d68bc8749eda0efb8c59af diff --git a/wallet/statements_gettextgen.po b/wallet/statements_gettextgen.po index a91c6b0ca..5fa433368 100644 --- a/wallet/statements_gettextgen.po +++ b/wallet/statements_gettextgen.po @@ -1238,11 +1238,11 @@ msgstr "" msgid "not a valid SQL statement" msgstr "" -#: wallet/test/run-wallet.c:1434 +#: wallet/test/run-wallet.c:1441 msgid "SELECT COUNT(1) FROM channel_funding_inflights WHERE channel_id = ?;" msgstr "" -#: wallet/test/run-wallet.c:1632 +#: wallet/test/run-wallet.c:1639 msgid "INSERT INTO channels (id) VALUES (1);" msgstr "" -# SHA256STAMP:6a7cda25fc90182775e1fd2a4055e2c1e74ddca42f8a0160fea20e7aa3d045cb +# SHA256STAMP:249962d1ad354071c65dcbdbabf051fde833e7c9b36f78b2db093d3209e419d5 diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 93da0057e..960d43c95 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -119,6 +119,9 @@ void fatal(const char *fmt UNNEEDED, ...) bool feature_negotiated(const struct feature_set *our_features UNNEEDED, const u8 *their_features UNNEEDED, size_t f UNNEEDED) { fprintf(stderr, "feature_negotiated called!\n"); abort(); } +/* Generated stub for feature_offered */ +bool feature_offered(const u8 *features UNNEEDED, size_t f UNNEEDED) +{ fprintf(stderr, "feature_offered called!\n"); abort(); } /* Generated stub for fromwire_channeld_dev_memleak_reply */ bool fromwire_channeld_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED) { fprintf(stderr, "fromwire_channeld_dev_memleak_reply called!\n"); abort(); } @@ -405,6 +408,10 @@ bool json_to_node_id(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, bool json_to_number(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, unsigned int *num UNNEEDED) { fprintf(stderr, "json_to_number called!\n"); abort(); } +/* Generated stub for json_to_outpoint */ +bool json_to_outpoint(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, + struct bitcoin_outpoint *op UNNEEDED) +{ fprintf(stderr, "json_to_outpoint called!\n"); abort(); } /* Generated stub for json_to_preimage */ bool json_to_preimage(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct preimage *preimage UNNEEDED) { fprintf(stderr, "json_to_preimage called!\n"); abort(); }