close: check that destination is going to be accepted.

Prior to this, sending a v1 address (or, in fact, any random crap!)
would cause the unsupporting node to unilaterally close.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2021-06-10 13:21:12 +09:30
parent 3cf98085d4
commit 064ad486e3
10 changed files with 67 additions and 9 deletions

7
doc/lightning-close.7 generated
View file

@ -27,7 +27,10 @@ The default is 2 days (172800 seconds)\.
The \fIdestination\fR can be of any Bitcoin accepted type, including bech32\. The \fIdestination\fR can be of any Bitcoin accepted type, including bech32\.
If it isn't specified, the default is a c-lightning wallet address\. If it isn't specified, the default is a c-lightning wallet address\. If
the peer hasn't offered the \fBoption_shutdown_anysegwit\fR feature, then
taproot addresses (or other v1+ segwit) are not allowed\. Tell your
friends to upgrade!
The \fIfee_negotiation_step\fR parameter controls how closing fee The \fIfee_negotiation_step\fR parameter controls how closing fee
@ -120,4 +123,4 @@ ZmnSCPxj \fI<ZmnSCPxj@protonmail.com\fR> is mainly responsible\.
Main web site: \fIhttps://github.com/ElementsProject/lightning\fR Main web site: \fIhttps://github.com/ElementsProject/lightning\fR
\" SHA256STAMP:507a9ca707e244eef65c5e16daa5a4d7ba8f59e93e988d252f7e854ae9f44781 \" SHA256STAMP:17f5bb362d8501b04314756c4134e3d5d20f8729dd55f5f3cfa0b5e111b104a1

View file

@ -26,7 +26,10 @@ indefinitely until the peer is online and can negotiate a mutual close.
The default is 2 days (172800 seconds). The default is 2 days (172800 seconds).
The *destination* can be of any Bitcoin accepted type, including bech32. The *destination* can be of any Bitcoin accepted type, including bech32.
If it isn't specified, the default is a c-lightning wallet address. If it isn't specified, the default is a c-lightning wallet address. If
the peer hasn't offered the `option_shutdown_anysegwit` feature, then
taproot addresses (or other v1+ segwit) are not allowed. Tell your
friends to upgrade!
The *fee_negotiation_step* parameter controls how closing fee The *fee_negotiation_step* parameter controls how closing fee
negotiation is performed assuming the peer proposes a fee that is negotiation is performed assuming the peer proposes a fee that is

View file

@ -244,7 +244,7 @@ static void peer_got_shutdown(struct channel *channel, const u8 *msg)
channel_fail_permanent(channel, channel_fail_permanent(channel,
REASON_PROTOCOL, REASON_PROTOCOL,
"Bad shutdown scriptpubkey %s", "Bad shutdown scriptpubkey %s",
tal_hex(channel, scriptpubkey)); tal_hex(tmpctx, scriptpubkey));
return; return;
} }

View file

@ -27,6 +27,7 @@
#include <common/key_derive.h> #include <common/key_derive.h>
#include <common/param.h> #include <common/param.h>
#include <common/per_peer_state.h> #include <common/per_peer_state.h>
#include <common/shutdown_scriptpubkey.h>
#include <common/status.h> #include <common/status.h>
#include <common/timeout.h> #include <common/timeout.h>
#include <common/utils.h> #include <common/utils.h>
@ -1655,6 +1656,7 @@ static struct command_result *json_close(struct command *cmd,
const char *fee_negotiation_step_str; const char *fee_negotiation_step_str;
struct bitcoin_outpoint *wrong_funding; struct bitcoin_outpoint *wrong_funding;
char* end; char* end;
bool anysegwit;
if (!param(cmd, buffer, params, if (!param(cmd, buffer, params,
p_req("id", param_tok, &idtok), p_req("id", param_tok, &idtok),
@ -1731,6 +1733,24 @@ static struct command_result *json_close(struct command *cmd,
} else } else
close_script_set = false; close_script_set = false;
/* Don't send a scriptpubkey peer won't accept */
anysegwit = feature_negotiated(cmd->ld->our_features,
channel->peer->their_features,
OPT_SHUTDOWN_ANYSEGWIT);
if (!valid_shutdown_scriptpubkey(channel->shutdown_scriptpubkey[LOCAL],
anysegwit)) {
/* Explicit check for future segwits. */
if (!anysegwit &&
valid_shutdown_scriptpubkey(channel->shutdown_scriptpubkey
[LOCAL], true)) {
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Peer does not allow v1+ shutdown addresses");
}
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Invalid close destination");
}
if (fee_negotiation_step_str == NULL) { if (fee_negotiation_step_str == NULL) {
channel->closing_fee_negotiation_step = 50; channel->closing_fee_negotiation_step = 50;
channel->closing_fee_negotiation_step_unit = channel->closing_fee_negotiation_step_unit =

View file

@ -670,6 +670,10 @@ u8 *towire_warningfmt(const tal_t *ctx UNNEEDED,
const struct channel_id *channel UNNEEDED, const struct channel_id *channel UNNEEDED,
const char *fmt UNNEEDED, ...) const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } { fprintf(stderr, "towire_warningfmt called!\n"); abort(); }
/* Generated stub for valid_shutdown_scriptpubkey */
bool valid_shutdown_scriptpubkey(const u8 *scriptpubkey UNNEEDED,
bool anysegwit UNNEEDED)
{ fprintf(stderr, "valid_shutdown_scriptpubkey called!\n"); abort(); }
/* Generated stub for version */ /* Generated stub for version */
const char *version(void) const char *version(void)
{ fprintf(stderr, "version called!\n"); abort(); } { fprintf(stderr, "version called!\n"); abort(); }

View file

@ -2772,3 +2772,27 @@ def test_segwit_anyshutdown(node_factory, bitcoind, executor):
l1.rpc.close(l2.info['id'], destination=addr) l1.rpc.close(l2.info['id'], destination=addr)
bitcoind.generate_block(1, wait_for_mempool=1) bitcoind.generate_block(1, wait_for_mempool=1)
wait_for(lambda: all([c['state'] == 'ONCHAIN' for c in only_one(l1.rpc.listpeers()['peers'])['channels']])) wait_for(lambda: all([c['state'] == 'ONCHAIN' for c in only_one(l1.rpc.listpeers()['peers'])['channels']]))
@pytest.mark.developer("needs to manipulate features")
@unittest.skipIf(TEST_NETWORK == 'liquid-regtest', "Uses regtest addresses")
def test_anysegwit_close_needs_feature(node_factory, bitcoind):
"""Rather than have peer reject our shutdown, we should refuse to shutdown toa v1+ address if they don't support it"""
# L2 says "no option_shutdown_anysegwit"
l1, l2 = node_factory.line_graph(2, opts=[{'may_reconnect': True},
{'may_reconnect': True,
'dev-force-features': -27}])
with pytest.raises(RpcError, match=r'Peer does not allow v1\+ shutdown addresses'):
l1.rpc.close(l2.info['id'], destination='bcrt1pw508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7k0ylj56')
# From TFM: "Tell your friends to upgrade!"
l2.stop()
del l2.daemon.opts['dev-force-features']
l2.start()
# Now it will work!
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
l1.rpc.close(l2.info['id'], destination='bcrt1pw508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7k0ylj56')
wait_for(lambda: only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])['state'] == 'CLOSINGD_COMPLETE')
bitcoind.generate_block(1, wait_for_mempool=1)

View file

@ -1924,4 +1924,4 @@ struct db_query db_postgres_queries[] = {
#endif /* LIGHTNINGD_WALLET_GEN_DB_POSTGRES */ #endif /* LIGHTNINGD_WALLET_GEN_DB_POSTGRES */
// SHA256STAMP:1379bcdee314439910fc6b238f7ec986536543c53933883ffd1b750dfc34f9b9 // SHA256STAMP:dbbcb7d784e7b3d6c7b27c2ff976dcc39335fdc26fbf095b65116488007799f7

View file

@ -1924,4 +1924,4 @@ struct db_query db_sqlite3_queries[] = {
#endif /* LIGHTNINGD_WALLET_GEN_DB_SQLITE3 */ #endif /* LIGHTNINGD_WALLET_GEN_DB_SQLITE3 */
// SHA256STAMP:1379bcdee314439910fc6b238f7ec986536543c53933883ffd1b750dfc34f9b9 // SHA256STAMP:dbbcb7d784e7b3d6c7b27c2ff976dcc39335fdc26fbf095b65116488007799f7

View file

@ -1262,11 +1262,11 @@ msgstr ""
msgid "not a valid SQL statement" msgid "not a valid SQL statement"
msgstr "" msgstr ""
#: wallet/test/run-wallet.c:1451 #: wallet/test/run-wallet.c:1455
msgid "SELECT COUNT(1) FROM channel_funding_inflights WHERE channel_id = ?;" msgid "SELECT COUNT(1) FROM channel_funding_inflights WHERE channel_id = ?;"
msgstr "" msgstr ""
#: wallet/test/run-wallet.c:1649 #: wallet/test/run-wallet.c:1653
msgid "INSERT INTO channels (id) VALUES (1);" msgid "INSERT INTO channels (id) VALUES (1);"
msgstr "" msgstr ""
# SHA256STAMP:51de0bd1efd4b12ec550d3faf934a32f745018a46e61b50cc242c2d5bae09470 # SHA256STAMP:e3c8d5cac8615668f0c9f37ebf6edff3b18833bafdf9643c2203b2a4ab654b7c

View file

@ -841,6 +841,10 @@ u8 *towire_warningfmt(const tal_t *ctx UNNEEDED,
const struct channel_id *channel UNNEEDED, const struct channel_id *channel UNNEEDED,
const char *fmt UNNEEDED, ...) const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); } { fprintf(stderr, "towire_warningfmt called!\n"); abort(); }
/* Generated stub for valid_shutdown_scriptpubkey */
bool valid_shutdown_scriptpubkey(const u8 *scriptpubkey UNNEEDED,
bool anysegwit UNNEEDED)
{ fprintf(stderr, "valid_shutdown_scriptpubkey called!\n"); abort(); }
/* Generated stub for watch_txid */ /* Generated stub for watch_txid */
struct txwatch *watch_txid(const tal_t *ctx UNNEEDED, struct txwatch *watch_txid(const tal_t *ctx UNNEEDED,
struct chain_topology *topo UNNEEDED, struct chain_topology *topo UNNEEDED,