From 48d9e3315462a49d184017442f29ddfe05cb66b0 Mon Sep 17 00:00:00 2001 From: ZmnSCPxj Date: Tue, 10 Apr 2018 06:03:15 +0000 Subject: [PATCH 1/4] peer_control: Make close wait for complete closure, with timeout. Also report tx and txid, and whether we closed unilaterally or bilaterally, if we could close the channel. Also make a manpage. Fixes: #1207 Fixes: #714 Fixes: #622 --- contrib/pylightning/lightning/lightning.py | 10 +- doc/Makefile | 1 + doc/lightning-close.7 | 59 +++++++ doc/lightning-close.7.txt | 70 ++++++++ lightningd/channel.c | 3 +- lightningd/closing_control.c | 3 +- lightningd/lightningd.c | 1 + lightningd/lightningd.h | 2 + lightningd/peer_control.c | 178 ++++++++++++++++++++- lightningd/peer_control.h | 2 +- tests/test_lightningd.py | 55 +++++-- wallet/test/run-wallet.c | 4 + 12 files changed, 360 insertions(+), 28 deletions(-) create mode 100644 doc/lightning-close.7 create mode 100644 doc/lightning-close.7.txt diff --git a/contrib/pylightning/lightning/lightning.py b/contrib/pylightning/lightning/lightning.py index c57a3dec1b13..2e89d3f0afee 100644 --- a/contrib/pylightning/lightning/lightning.py +++ b/contrib/pylightning/lightning/lightning.py @@ -333,12 +333,16 @@ def fundchannel(self, channel_id, satoshi): } return self.call("fundchannel", payload) - def close(self, peer_id): + def close(self, peer_id, force=None, timeout=None): """ - Close the channel with peer {id} + Close the channel with peer {id}, forcing a unilateral + close if {force} is True, and timing out with {timeout} + seconds. """ payload = { - "id": peer_id + "id": peer_id, + "force": force, + "timeout": timeout } return self.call("close", payload) diff --git a/doc/Makefile b/doc/Makefile index 788793e0b098..9acb59785079 100644 --- a/doc/Makefile +++ b/doc/Makefile @@ -6,6 +6,7 @@ doc-wrongdir: MANPAGES := doc/lightning-cli.1 \ doc/lightning-autocleaninvoice.7 \ + doc/lightning-close.7 \ doc/lightning-decodepay.7 \ doc/lightning-delexpiredinvoice.7 \ doc/lightning-delinvoice.7 \ diff --git a/doc/lightning-close.7 b/doc/lightning-close.7 new file mode 100644 index 000000000000..275f1107416d --- /dev/null +++ b/doc/lightning-close.7 @@ -0,0 +1,59 @@ +'\" t +.\" Title: lightning-close +.\" Author: [see the "AUTHOR" section] +.\" Generator: DocBook XSL Stylesheets v1.79.1 +.\" Date: 04/15/2018 +.\" Manual: \ \& +.\" Source: \ \& +.\" Language: English +.\" +.TH "LIGHTNING\-CLOSE" "7" "04/15/2018" "\ \&" "\ \&" +.\" ----------------------------------------------------------------- +.\" * Define some portability stuff +.\" ----------------------------------------------------------------- +.\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +.\" http://bugs.debian.org/507673 +.\" http://lists.gnu.org/archive/html/groff/2009-02/msg00013.html +.\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +.ie \n(.g .ds Aq \(aq +.el .ds Aq ' +.\" ----------------------------------------------------------------- +.\" * set default formatting +.\" ----------------------------------------------------------------- +.\" disable hyphenation +.nh +.\" disable justification (adjust text to left margin only) +.ad l +.\" ----------------------------------------------------------------- +.\" * MAIN CONTENT STARTS HERE * +.\" ----------------------------------------------------------------- +.SH "NAME" +lightning-close \- Protocol for closing channels with direct peers +.SH "SYNOPSIS" +.sp +\fBclose\fR \fIid\fR [\fIforce\fR] [\fItimeout\fR] +.SH "DESCRIPTION" +.sp +The \fBclose\fR RPC command attempts to close the channel cooperatively with the peer\&. It applies to the active channel of the direct peer corresponding to the given peer \fIid\fR\&. +.sp +The \fBclose\fR command will time out and return with an error when the number of seconds specified in \fItimeout\fR is reached\&. If unspecified, it times out in 30 seconds\&. +.sp +The \fIforce\fR argument, if the JSON value \fItrue\fR, will cause the channel to be unilaterally closed when the timeout is reached\&. If so, timeout will not cause an error, but instead cause the channel to be failed and put onchain unilaterally\&. Unilateral closes will lead to your funds getting locked according to the \fIto_self_delay\fR parameter of the peer\&. +.sp +Normally the peer needs to be live and connected in order to negotiate a mutual close\&. Forcing a unilateral close can be used if you suspect you can no longer contact the peer\&. +.SH "RETURN VALUE" +.sp +On success, an object with fields \fItx\fR and \fItxid\fR containing the closing transaction are returned\&. It will also have a field \fItype\fR which is either the JSON string \fImutual\fR or the JSON string \fIunilateral\fR\&. A \fImutual\fR close means that we could negotiate a close with the peer, while a \fIunilateral\fR close means that the \fIforce\fR flag was set and we had to close the channel without waiting for the counterparty\&. +.sp +A unilateral close may still occur with \fIforce\fR set to \fIfalse\fR if the peer did not behave correctly during the close negotiation\&. +.sp +Unilateral closes will return your funds after a delay\&. The delay will vary based on the peer \fIto_self_delay\fR setting, not your own setting\&. +.sp +On failure, if \fBclose\fR failed due to timing out with \fIforce\fR argument \fIfalse\fR, the channel will still eventually close once we have contacted the peer\&. +.SH "AUTHOR" +.sp +ZmnSCPxj is mainly responsible\&. +.SH "SEE ALSO" +.SH "RESOURCES" +.sp +Main web site: https://github\&.com/ElementsProject/lightning diff --git a/doc/lightning-close.7.txt b/doc/lightning-close.7.txt new file mode 100644 index 000000000000..a3ef5170e417 --- /dev/null +++ b/doc/lightning-close.7.txt @@ -0,0 +1,70 @@ +LIGHTNING-CLOSE(7) +================== +:doctype: manpage + +NAME +---- +lightning-close - Protocol for closing channels with direct peers + +SYNOPSIS +-------- +*close* 'id' ['force'] ['timeout'] + +DESCRIPTION +----------- + +The *close* RPC command attempts to close the channel cooperatively +with the peer. +It applies to the active channel of the direct peer corresponding to +the given peer 'id'. + +The *close* command will time out and return with an error when the +number of seconds specified in 'timeout' is reached. +If unspecified, it times out in 30 seconds. + +The 'force' argument, if the JSON value 'true', will cause the +channel to be unilaterally closed when the timeout is reached. +If so, timeout will not cause an error, but instead cause the +channel to be failed and put onchain unilaterally. +Unilateral closes will lead to your funds getting locked according +to the 'to_self_delay' parameter of the peer. + +Normally the peer needs to be live and connected in order to negotiate +a mutual close. +Forcing a unilateral close can be used if you suspect you can no longer +contact the peer. + +RETURN VALUE +------------ + +On success, an object with fields 'tx' and 'txid' containing the +closing transaction are returned. +It will also have a field 'type' which is either the JSON string +'mutual' or the JSON string 'unilateral'. +A 'mutual' close means that we could negotiate a close with the +peer, while a 'unilateral' close means that the 'force' flag was +set and we had to close the channel without waiting for the +counterparty. + +A unilateral close may still occur with 'force' set to 'false' if +the peer did not behave correctly during the close negotiation. + +Unilateral closes will return your funds after a delay. +The delay will vary based on the peer 'to_self_delay' setting, not +your own setting. + +On failure, if *close* failed due to timing out with 'force' +argument 'false', the channel will still eventually close once +we have contacted the peer. + +AUTHOR +------ +ZmnSCPxj is mainly responsible. + +SEE ALSO +-------- + + +RESOURCES +--------- +Main web site: https://github.com/ElementsProject/lightning diff --git a/lightningd/channel.c b/lightningd/channel.c index 87325a885471..a87bc1d9d437 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -321,7 +321,8 @@ void channel_fail_permanent(struct channel *channel, const char *fmt, ...) } channel_set_owner(channel, NULL); - drop_to_chain(ld, channel); + /* Drop non-cooperatively (unilateral) to chain. */ + drop_to_chain(ld, channel, false); tal_free(why); } diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index d0f9fa11358d..e6296ec358a3 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -93,7 +93,8 @@ static void peer_closing_complete(struct channel *channel, const u8 *msg) if (channel->state == CLOSINGD_COMPLETE) return; - drop_to_chain(channel->peer->ld, channel); + /* Channel gets dropped to chain cooperatively. */ + drop_to_chain(channel->peer->ld, channel, true); channel_set_state(channel, CLOSINGD_SIGEXCHANGE, CLOSINGD_COMPLETE); } diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index d671cf712780..18976281814a 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -68,6 +68,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx) list_head_init(&ld->connects); list_head_init(&ld->waitsendpay_commands); list_head_init(&ld->sendpay_commands); + list_head_init(&ld->close_commands); ld->wireaddrs = tal_arr(ld, struct wireaddr, 0); ld->portnum = DEFAULT_PORT; timers_init(&ld->timers, time_mono()); diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index f1e9ebce912f..2ff947202c10 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -138,6 +138,8 @@ struct lightningd { struct list_head waitsendpay_commands; /* Outstanding sendpay commands. */ struct list_head sendpay_commands; + /* Outstanding close commands. */ + struct list_head close_commands; /* Maintained by invoices.c */ struct invoices *invoices; diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 820b4d3be6f6..984811f64246 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -44,6 +44,17 @@ #include #include +struct close_command { + /* Inside struct lightningd close_commands. */ + struct list_node list; + /* Command structure. This is the parent of the close command. */ + struct command *cmd; + /* Channel being closed. */ + struct channel *channel; + /* Should we force the close on timeout? */ + bool force; +}; + static void destroy_peer(struct peer *peer) { list_del_from(&peer->ld->peers, &peer->list); @@ -209,13 +220,126 @@ static void remove_sig(struct bitcoin_tx *signed_tx) signed_tx->input[0].witness = tal_free(signed_tx->input[0].witness); } -void drop_to_chain(struct lightningd *ld, struct channel *channel) +/* Resolve a single close command. */ +static void +resolve_one_close_command(struct close_command *cc, bool cooperative) +{ + struct json_result *result = new_json_result(cc); + u8 *tx = linearize_tx(result, cc->channel->last_tx); + struct bitcoin_txid txid; + + bitcoin_txid(cc->channel->last_tx, &txid); + + json_object_start(result, NULL); + json_add_hex(result, "tx", tx, tal_len(tx)); + json_add_txid(result, "txid", &txid); + if (cooperative) + json_add_string(result, "type", "mutual"); + else + json_add_string(result, "type", "unilateral"); + json_object_end(result); + + command_success(cc->cmd, result); +} + +/* Resolve a close command for a channel that will be closed soon. */ +static void +resolve_close_command(struct lightningd *ld, struct channel *channel, + bool cooperative) +{ + struct close_command *cc; + struct close_command *n; + + list_for_each_safe (&ld->close_commands, cc, n, list) { + if (cc->channel != channel) + continue; + resolve_one_close_command(cc, cooperative); + } +} + +/* Destroy the close command structure in reaction to the + * channel being destroyed. */ +static void +destroy_close_command_on_channel_destroy(struct channel *_ UNUSED, + struct close_command *cc) +{ + /* The cc has the command as parent, so resolving the + * command destroys the cc and triggers destroy_close_command. + * Clear the cc->channel first so that we will not try to + * remove a destructor. */ + cc->channel = NULL; + command_fail(cc->cmd, "Channel forgotten before proper close."); +} + +/* Destroy the close command structure. */ +static void +destroy_close_command(struct close_command *cc) +{ + list_del(&cc->list); + /* If destroy_close_command_on_channel_destroy was + * triggered beforehand, it will have cleared + * the channel field, preventing us from removing it + * from an already-destroyed channel. */ + if (!cc->channel) + return; + tal_del_destructor2(cc->channel, + &destroy_close_command_on_channel_destroy, + cc); +} + +/* Handle timeout. */ +static void +close_command_timeout(struct close_command *cc) +{ + if (cc->force) + /* This will trigger drop_to_chain, which will trigger + * resolution of the command and destruction of the + * close_command. */ + channel_fail_permanent(cc->channel, + "Forcibly closed by 'close' command timeout"); + else + /* Fail the command directly, which will resolve the + * command and destroy the close_command. */ + command_fail(cc->cmd, + "Channel close negotiation not finished " + "before timeout"); +} + +/* Construct a close command structure and add to ld. */ +static void +register_close_command(struct lightningd *ld, + struct command *cmd, + struct channel *channel, + unsigned int timeout, + bool force) +{ + struct close_command *cc; + assert(channel); + + cc = tal(cmd, struct close_command); + list_add_tail(&ld->close_commands, &cc->list); + cc->cmd = cmd; + cc->channel = channel; + cc->force = force; + tal_add_destructor(cc, &destroy_close_command); + tal_add_destructor2(channel, + &destroy_close_command_on_channel_destroy, + cc); + new_reltimer(&ld->timers, cc, time_from_sec(timeout), + &close_command_timeout, cc); +} + +void drop_to_chain(struct lightningd *ld, struct channel *channel, + bool cooperative) { sign_last_tx(channel); /* Keep broadcasting until we say stop (can fail due to dup, * if they beat us to the broadcast). */ broadcast_tx(ld->topology, channel, channel->last_tx, NULL); + + resolve_close_command(ld, channel, cooperative); + remove_sig(channel->last_tx); } @@ -854,11 +978,17 @@ static void json_close(struct command *cmd, const char *buffer, const jsmntok_t *params) { jsmntok_t *peertok; + jsmntok_t *timeouttok; + jsmntok_t *forcetok; struct peer *peer; struct channel *channel; + unsigned int timeout = 30; + bool force = false; if (!json_get_params(cmd, buffer, params, "id", &peertok, + "?force", &forcetok, + "?timeout", &timeouttok, NULL)) { return; } @@ -868,6 +998,18 @@ static void json_close(struct command *cmd, command_fail(cmd, "Could not find peer with that id"); return; } + if (forcetok && !json_tok_bool(buffer, forcetok, &force)) { + command_fail(cmd, "Force '%.*s' must be true or false", + forcetok->end - forcetok->start, + buffer + forcetok->start); + return; + } + if (timeouttok && !json_tok_number(buffer, timeouttok, &timeout)) { + command_fail(cmd, "Timeout '%.*s' is not a number", + timeouttok->end - timeouttok->start, + buffer + timeouttok->start); + return; + } channel = peer_active_channel(peer); if (!channel) { @@ -883,7 +1025,22 @@ static void json_close(struct command *cmd, return; } - /* Normal case. */ + /* Normal case. + * We allow states shutting down and sigexchange; a previous + * close command may have timed out, and this current command + * will continue waiting for the effects of the previous + * close command. */ + if (channel->state != CHANNELD_NORMAL && + channel->state != CHANNELD_AWAITING_LOCKIN && + channel->state != CHANNELD_SHUTTING_DOWN && + channel->state != CLOSINGD_SIGEXCHANGE) + command_fail(cmd, "Peer is in state %s", + channel_state_name(channel)); + + /* If normal or locking in, transition to shutting down + * state. + * (if already shutting down or sigexchange, just keep + * waiting) */ if (channel->state == CHANNELD_NORMAL || channel->state == CHANNELD_AWAITING_LOCKIN) { channel_set_state(channel, channel->state, CHANNELD_SHUTTING_DOWN); @@ -891,11 +1048,20 @@ static void json_close(struct command *cmd, if (channel->owner) subd_send_msg(channel->owner, take(towire_channel_send_shutdown(channel))); + } + /* If channel has no owner, it means the peer is disconnected, + * so make a nominal effort to contact it now. + */ + if (!channel->owner) + subd_send_msg(cmd->ld->gossip, + take(towire_gossipctl_reach_peer(NULL, + &channel->peer->id))); - command_success(cmd, null_response(cmd)); - } else - command_fail(cmd, "Peer is in state %s", - channel_state_name(channel)); + /* Register this command for later handling. */ + register_close_command(cmd->ld, cmd, channel, timeout, force); + + /* Wait until close drops down to chain. */ + command_still_pending(cmd); } static const struct json_command close_command = { diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index 44adc202a70c..dd8b524f11d7 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -102,7 +102,7 @@ u8 *p2wpkh_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx); /* We've loaded peers from database, set them going. */ void activate_peers(struct lightningd *ld); -void drop_to_chain(struct lightningd *ld, struct channel *channel); +void drop_to_chain(struct lightningd *ld, struct channel *channel, bool cooperative); /* Get range of feerates to insist other side abide by for normal channels. */ u32 feerate_min(struct lightningd *ld); diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index ef5770a50471..8157c8b8d2ce 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -1180,8 +1180,10 @@ def test_closing(self): billboard = l1.rpc.listpeers(l2.info['id'])['peers'][0]['channels'][0]['status'] assert billboard == ['CHANNELD_NORMAL:Funding transaction locked. Channel announced.'] - # This should return, then close. - l1.rpc.close(l2.info['id']) + # This should return with an error, then close. + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l2.info['id'], False, 0) l1.daemon.wait_for_log(' to CHANNELD_SHUTTING_DOWN') l2.daemon.wait_for_log(' to CHANNELD_SHUTTING_DOWN') @@ -1306,7 +1308,9 @@ def test_closing_different_fees(self): # Now close for p in peers: - l1.rpc.close(p.info['id']) + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, p.info['id'], False, 0) for p in peers: p.daemon.wait_for_log(' to CLOSINGD_COMPLETE') @@ -3088,8 +3092,10 @@ def test_shutdown_reconnect(self): assert l1.bitcoin.rpc.getmempoolinfo()['size'] == 0 - # This should return, then close. - l1.rpc.close(l2.info['id']) + # This should return with an error, then close. + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l2.info['id'], False, 0) l1.daemon.wait_for_log(' to CHANNELD_SHUTTING_DOWN') l2.daemon.wait_for_log(' to CHANNELD_SHUTTING_DOWN') @@ -3114,7 +3120,10 @@ def test_shutdown_awaiting_lockin(self): l1.daemon.wait_for_log('sendrawtx exit 0') bitcoind.generate_block(1) - l1.rpc.close(l2.info['id']) + # This should return with an error, then close. + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l2.info['id'], False, 0) l1.daemon.wait_for_log('CHANNELD_AWAITING_LOCKIN to CHANNELD_SHUTTING_DOWN') l2.daemon.wait_for_log('CHANNELD_AWAITING_LOCKIN to CHANNELD_SHUTTING_DOWN') @@ -3149,8 +3158,10 @@ def test_closing_negotiation_reconnect(self): assert l1.bitcoin.rpc.getmempoolinfo()['size'] == 0 - # This should return, then close. - l1.rpc.close(l2.info['id']) + # This should return with an error, then close. + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l2.info['id'], False, 0) l1.daemon.wait_for_log(' to CHANNELD_SHUTTING_DOWN') l2.daemon.wait_for_log(' to CHANNELD_SHUTTING_DOWN') @@ -3808,7 +3819,9 @@ def test_update_fee(self): self.pay(l2, l1, 100000000) # Now shutdown cleanly. - l1.rpc.close(l2.info['id']) + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l2.info['id'], False, 0) l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE') l2.daemon.wait_for_log(' to CLOSINGD_COMPLETE') @@ -3852,8 +3865,10 @@ def test_update_all_fees(self): l1.rpc.dev_setfees() l1.daemon.wait_for_log('dev-setfees: fees now 21098/7654/321') - # Now shutdown cleanly. - l1.rpc.close(l2.info['id']) + # This should return with an error, then close. + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l2.info['id'], False, 0) l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE') l2.daemon.wait_for_log(' to CLOSINGD_COMPLETE') @@ -3892,8 +3907,10 @@ def test_fee_limits(self): # 15sat/byte fee l1.daemon.wait_for_log('peer_out WIRE_REVOKE_AND_ACK') - # Now shutdown cleanly. - l1.rpc.close(l3.info['id']) + # This should return with an error, then close. + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l3.info['id'], False, 0) l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE') l3.daemon.wait_for_log(' to CLOSINGD_COMPLETE') @@ -3924,7 +3941,9 @@ def test_update_fee_reconnect(self): assert l2.daemon.is_in_log('got commitsig [0-9]*: feerate 14000') # Now shutdown cleanly. - l1.rpc.close(l2.info['id']) + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l2.info['id'], False, 0) l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE') l2.daemon.wait_for_log(' to CLOSINGD_COMPLETE') @@ -4060,7 +4079,9 @@ def test_multiple_channels(self): l2.daemon.wait_for_log('Handing back peer .* to master') self.fund_channel(l1, l2, 10**6) - l1.rpc.close(l2.info['id']) + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l2.info['id'], False, 0) l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE') l2.daemon.wait_for_log(' to CLOSINGD_COMPLETE') @@ -4205,7 +4226,9 @@ def test_peerinfo(self): assert l1.rpc.getpeer(l2.info['id'])['color'] == l1.rpc.listnodes(l2.info['id'])['nodes'][0]['color'] # Close the channel to forget the peer - l1.rpc.close(l2.info['id']) + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l2.info['id'], False, 0) l1.daemon.wait_for_log('Forgetting remote peer') bitcoind.generate_block(100) l1.daemon.wait_for_log('WIRE_ONCHAIN_ALL_IRREVOCABLY_RESOLVED') diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index e132ea611240..8a4b5eb4522d 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -253,6 +253,10 @@ bool json_tok_bool(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, b bool json_tok_loglevel(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, enum log_level *level UNNEEDED) { fprintf(stderr, "json_tok_loglevel called!\n"); abort(); } +/* Generated stub for json_tok_number */ +bool json_tok_number(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, + unsigned int *num UNNEEDED) +{ fprintf(stderr, "json_tok_number called!\n"); abort(); } /* Generated stub for json_tok_pubkey */ bool json_tok_pubkey(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct pubkey *pubkey UNNEEDED) From 443fedde2fd9148b22c02d95151248ef72be2d12 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Apr 2018 15:34:53 +0930 Subject: [PATCH 2/4] json_close: test that it works while disconnected. It should, indeed, close once they reconnect. Signed-off-by: Rusty Russell --- tests/test_lightningd.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 8157c8b8d2ce..0fd4f9d35f4e 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -1224,6 +1224,32 @@ def test_closing(self): wait_forget_channels(l1) wait_forget_channels(l2) + def test_closing_while_disconnected(self): + l1, l2 = self.connect() + + self.fund_channel(l1, l2, 10**6) + self.pay(l1, l2, 200000000) + + l2.stop() + + # The close should still be triggered afterwards. + self.assertRaisesRegex(ValueError, + "Channel close negotiation not finished", + l1.rpc.close, l2.info['id'], False, 0) + l1.daemon.wait_for_log(' to CHANNELD_SHUTTING_DOWN') + + l2.daemon.start() + l1.daemon.wait_for_log(' to CLOSINGD_SIGEXCHANGE') + l2.daemon.wait_for_log(' to CLOSINGD_SIGEXCHANGE') + + # And should put closing into mempool. + l1.daemon.wait_for_log('sendrawtx exit 0') + l2.daemon.wait_for_log('sendrawtx exit 0') + + bitcoind.rpc.generate(101) + wait_forget_channels(l1) + wait_forget_channels(l2) + def test_db_upgrade(self): l1 = self.node_factory.get_node() l1.stop() From 28868b8c6e8e9ae2b4755f742ac2f6165e97d242 Mon Sep 17 00:00:00 2001 From: ZmnSCPxj Date: Mon, 16 Apr 2018 11:29:39 +0000 Subject: [PATCH 3/4] test_lightningd.py: Clean up some uses of 'close' RPC. --- tests/test_lightningd.py | 37 +++++++++---------------------------- 1 file changed, 9 insertions(+), 28 deletions(-) diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 0fd4f9d35f4e..957aeaf17a15 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -1333,13 +1333,10 @@ def test_closing_different_fees(self): self.pay(l1, p, 100000000) # Now close - for p in peers: - self.assertRaisesRegex(ValueError, - "Channel close negotiation not finished", - l1.rpc.close, p.info['id'], False, 0) + closes = [self.executor.submit(l1.rpc.close, p.info['id']) for p in peers] - for p in peers: - p.daemon.wait_for_log(' to CLOSINGD_COMPLETE') + for c in closes: + c.result(30) bitcoind.generate_block(1) for p in peers: @@ -3891,12 +3888,8 @@ def test_update_all_fees(self): l1.rpc.dev_setfees() l1.daemon.wait_for_log('dev-setfees: fees now 21098/7654/321') - # This should return with an error, then close. - self.assertRaisesRegex(ValueError, - "Channel close negotiation not finished", - l1.rpc.close, l2.info['id'], False, 0) - l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE') - l2.daemon.wait_for_log(' to CLOSINGD_COMPLETE') + # This should return finish closing. + l1.rpc.close(l2.info['id']) @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") def test_fee_limits(self): @@ -3933,12 +3926,8 @@ def test_fee_limits(self): # 15sat/byte fee l1.daemon.wait_for_log('peer_out WIRE_REVOKE_AND_ACK') - # This should return with an error, then close. - self.assertRaisesRegex(ValueError, - "Channel close negotiation not finished", - l1.rpc.close, l3.info['id'], False, 0) - l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE') - l3.daemon.wait_for_log(' to CLOSINGD_COMPLETE') + # This should wait for close to complete + l1.rpc.close(l3.info['id']) @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") def test_update_fee_reconnect(self): @@ -3967,11 +3956,7 @@ def test_update_fee_reconnect(self): assert l2.daemon.is_in_log('got commitsig [0-9]*: feerate 14000') # Now shutdown cleanly. - self.assertRaisesRegex(ValueError, - "Channel close negotiation not finished", - l1.rpc.close, l2.info['id'], False, 0) - l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE') - l2.daemon.wait_for_log(' to CLOSINGD_COMPLETE') + l1.rpc.close(l2.info['id']) # And should put closing into mempool. l1.daemon.wait_for_log('sendrawtx exit 0') @@ -4105,11 +4090,7 @@ def test_multiple_channels(self): l2.daemon.wait_for_log('Handing back peer .* to master') self.fund_channel(l1, l2, 10**6) - self.assertRaisesRegex(ValueError, - "Channel close negotiation not finished", - l1.rpc.close, l2.info['id'], False, 0) - l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE') - l2.daemon.wait_for_log(' to CLOSINGD_COMPLETE') + l1.rpc.close(l2.info['id']) channels = l1.rpc.listpeers()['peers'][0]['channels'] assert len(channels) == 3 From fa3cff661a6017626fd784dca346c0237d90ad56 Mon Sep 17 00:00:00 2001 From: "ZmnSCPxj, ZmnSCPxj jxPCSmnZ" Date: Wed, 18 Apr 2018 09:57:02 +0800 Subject: [PATCH 4/4] Loosen` close` timeout in `test_closing_different_fees` --- tests/test_lightningd.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 957aeaf17a15..3c32914ae9f1 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -1333,10 +1333,16 @@ def test_closing_different_fees(self): self.pay(l1, p, 100000000) # Now close - closes = [self.executor.submit(l1.rpc.close, p.info['id']) for p in peers] + # All closes occur in parallel, and on Travis, + # ALL those lightningd are running on a single core, + # so increase the timeout so that this test will pass + # when valgrind is enabled. + # (close timeout defaults to 30 as of this writing, more + # than double the default) + closes = [self.executor.submit(l1.rpc.close, p.info['id'], False, 72) for p in peers] for c in closes: - c.result(30) + c.result(72) bitcoind.generate_block(1) for p in peers: