Skip to content

Commit

Permalink
close: check that destination is going to be accepted.
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
rustyrussell committed Jun 9, 2021
1 parent a8cd35a commit 555f60c
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 9 deletions.
7 changes: 5 additions & 2 deletions doc/lightning-close.7

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion doc/lightning-close.7.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ indefinitely until the peer is online and can negotiate a mutual close.
The default is 2 days (172800 seconds).

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
negotiation is performed assuming the peer proposes a fee that is
Expand Down
2 changes: 1 addition & 1 deletion lightningd/channel_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ static void peer_got_shutdown(struct channel *channel, const u8 *msg)
channel_fail_permanent(channel,
REASON_PROTOCOL,
"Bad shutdown scriptpubkey %s",
tal_hex(channel, scriptpubkey));
tal_hex(tmpctx, scriptpubkey));
return;
}

Expand Down
20 changes: 20 additions & 0 deletions lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <common/key_derive.h>
#include <common/param.h>
#include <common/per_peer_state.h>
#include <common/shutdown_scriptpubkey.h>
#include <common/status.h>
#include <common/timeout.h>
#include <common/utils.h>
Expand Down Expand Up @@ -1655,6 +1656,7 @@ static struct command_result *json_close(struct command *cmd,
const char *fee_negotiation_step_str;
struct bitcoin_outpoint *wrong_funding;
char* end;
bool anysegwit;

if (!param(cmd, buffer, params,
p_req("id", param_tok, &idtok),
Expand Down Expand Up @@ -1731,6 +1733,24 @@ static struct command_result *json_close(struct command *cmd,
} else
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) {
channel->closing_fee_negotiation_step = 50;
channel->closing_fee_negotiation_step_unit =
Expand Down
4 changes: 4 additions & 0 deletions lightningd/test/run-invoice-select-inchan.c
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,10 @@ u8 *towire_warningfmt(const tal_t *ctx UNNEEDED,
const struct channel_id *channel UNNEEDED,
const char *fmt UNNEEDED, ...)
{ 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 */
const char *version(void)
{ fprintf(stderr, "version called!\n"); abort(); }
Expand Down
23 changes: 23 additions & 0 deletions tests/test_closing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2771,3 +2771,26 @@ def test_segwit_anyshutdown(node_factory, bitcoind, executor):
l1.rpc.close(l2.info['id'], destination=addr)
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']]))


@pytest.mark.developer("needs to manipulate features")
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)
2 changes: 1 addition & 1 deletion wallet/db_postgres_sqlgen.c

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion wallet/db_sqlite3_sqlgen.c

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions wallet/statements_gettextgen.po

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions wallet/test/run-wallet.c
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,10 @@ u8 *towire_warningfmt(const tal_t *ctx UNNEEDED,
const struct channel_id *channel UNNEEDED,
const char *fmt UNNEEDED, ...)
{ 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 */
struct txwatch *watch_txid(const tal_t *ctx UNNEEDED,
struct chain_topology *topo UNNEEDED,
Expand Down

0 comments on commit 555f60c

Please sign in to comment.