From 17d94845454638c4dca79c5a5a8c2feab2fe4a26 Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 29 Jul 2023 20:43:28 -0500 Subject: [PATCH 1/3] funder: don't re-reserve utxos on retries This way unreserving the PSBT will work as intended, and we don't have to keep track of how many times we've called reserved for any one input. Technically we're supposed to not reserve inputs at *all* while doing opens, this moves us slightly closer to that. --- plugins/funder.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/funder.c b/plugins/funder.c index 478a99f9c314..5e08370dd3f0 100644 --- a/plugins/funder.c +++ b/plugins/funder.c @@ -695,6 +695,8 @@ listfunds_success(struct command *cmd, committed_funds, avail_utxos); json_add_bool(req->js, "reservedok", true); + /* We don't re-reserve any UTXOS :) */ + json_add_num(req->js, "reserve", 0); } else { req = jsonrpc_request_start(cmd->plugin, cmd, "fundpsbt", From 6912ac25676a78e63e2aa0bdd8f328450b4031a7 Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 29 Jul 2023 15:48:43 -0500 Subject: [PATCH 2/3] dual-fund tests: add tests for reported incompat with Eclair Bug Report: - initiate a channel open eclair -> cln - wait for the transaction to be published - eclair initiates rbf, and cancels it by sending tx_abort before exchanging commit_sig - at that point everything looks good, cln echoes the tx_abort and stays connected - eclair initiates another RBF attempt and sends tx_init_rbf: for some unknown reason, cln answers with channel_reestablish (??) followed by an error saying "Bad reestablish message: WIRE_TX_INIT_RBF" Diagnosis: CLN is doing a reconnect after a tx-abort is sent. Extra Test: Realized that if we abort, we won't correctly advanced to NORMAL if blocks are mined while we're in hanging state. CLN should advance after block containing channel open is mined. Reported-By: @t-bast --- tests/test_opening.py | 144 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/tests/test_opening.py b/tests/test_opening.py index 45e558c671d9..e7dfc5955486 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -378,6 +378,150 @@ def test_v2_rbf_single(node_factory, bitcoind, chainparams): l1.daemon.wait_for_log('sendrawtx exit 0') +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') +@pytest.mark.openchannel('v2') +@pytest.mark.xfail +def test_v2_rbf_abort_retry(node_factory, bitcoind, chainparams): + l1, l2 = node_factory.get_nodes(2, opts={'wumbo': None, + 'allow_warning': True}) + + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + amount = 2**24 + chan_amount = 100000 + bitcoind.rpc.sendmany("", + {l1.rpc.newaddr()['bech32']: amount / 10**8 + 0.01, + l2.rpc.newaddr()['bech32']: amount / 10**8 + 0.01}) + bitcoind.generate_block(1) + # Wait for it to arrive. + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) > 0) + wait_for(lambda: len(l2.rpc.listfunds()['outputs']) > 0) + + l1_utxos = ['{}:{}'.format(utxo['txid'], utxo['output']) for utxo in l1.rpc.listfunds()['outputs']] + + # setup l2 to dual-fund! + l2.rpc.call('funderupdate', {'policy': 'match', + 'policy_mod': 100, + 'leases_only': False}) + + res = l1.rpc.fundchannel(l2.info['id'], chan_amount) + chan_id = res['channel_id'] + vins = bitcoind.rpc.decoderawtransaction(res['tx'])['vin'] + prev_utxos = ["{}:{}".format(vin['txid'], vin['vout']) for vin in vins if "{}:{}".format(vin['txid'], vin['vout']) in l1_utxos] + + # Check that we're waiting for lockin + l1.daemon.wait_for_log(' to DUALOPEND_AWAITING_LOCKIN') + + # Check that feerate info is correct + info_1 = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels']) + next_rate = "{}perkw".format(info_1['next_feerate'][:-5]) + + # Initiate an RBF + startweight = 42 + 172 # base weight, funding output + initpsbt = l1.rpc.utxopsbt(chan_amount, next_rate, startweight, + prev_utxos, reservedok=True, + min_witness_weight=110, + excess_as_change=True) + + # Do the bump + bump = l1.rpc.openchannel_bump(chan_id, chan_amount, initpsbt['psbt']) + + # We abort the channel mid-way throught the RBF + l1.rpc.openchannel_abort(chan_id) + + with pytest.raises(RpcError): + l1.rpc.openchannel_update(chan_id, bump['psbt']) + + # - initiate a channel open eclair -> cln + # - wait for the transaction to be published + # - eclair initiates rbf, and cancels it by sending tx_abort before exchanging commit_sig + # - at that point everything looks good, cln echoes the tx_abort and stays connected + # - eclair initiates another RBF attempt and sends tx_init_rbf: for some unknown reason, cln answers with channel_reestablish (??) followed by an error saying "Bad reestablish message: WIRE_TX_INIT_RBF" + + # attempt to initiate an RBF again + info_1 = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels']) + next_rate = "{}perkw".format(int(info_1['next_feerate'][:-5]) * 2) + + # Gotta unreserve the psbt and re-reserve with higher feerate + l1.rpc.unreserveinputs(initpsbt['psbt']) + initpsbt = l1.rpc.utxopsbt(chan_amount, next_rate, startweight, + prev_utxos, reservedok=True, + min_witness_weight=110, + excess_as_change=True) + # Do the bump+sign + bump = l1.rpc.openchannel_bump(chan_id, chan_amount, initpsbt['psbt'], + funding_feerate=next_rate) + + update = l1.rpc.openchannel_update(chan_id, bump['psbt']) + signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt'] + l1.rpc.openchannel_signed(chan_id, signed_psbt) + + bitcoind.generate_block(1) + sync_blockheight(bitcoind, [l1]) + l1.daemon.wait_for_log(' to CHANNELD_NORMAL') + assert not l1.daemon.is_in_log('WIRE_CHANNEL_REESTABLISH') + assert not l2.daemon.is_in_log('WIRE_CHANNEL_REESTABLISH') + + +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') +@pytest.mark.xfail +@pytest.mark.openchannel('v2') +def test_v2_rbf_abort_channel_opens(node_factory, bitcoind, chainparams): + l1, l2 = node_factory.get_nodes(2, opts={'wumbo': None, + 'allow_warning': True}) + + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + amount = 2**24 + chan_amount = 100000 + bitcoind.rpc.sendmany("", + {l1.rpc.newaddr()['bech32']: amount / 10**8 + 0.01, + l2.rpc.newaddr()['bech32']: amount / 10**8 + 0.01}) + bitcoind.generate_block(1) + # Wait for it to arrive. + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) > 0) + wait_for(lambda: len(l2.rpc.listfunds()['outputs']) > 0) + + l1_utxos = ['{}:{}'.format(utxo['txid'], utxo['output']) for utxo in l1.rpc.listfunds()['outputs']] + + # setup l2 to dual-fund! + l2.rpc.call('funderupdate', {'policy': 'match', + 'policy_mod': 100, + 'leases_only': False}) + + res = l1.rpc.fundchannel(l2.info['id'], chan_amount) + chan_id = res['channel_id'] + vins = bitcoind.rpc.decoderawtransaction(res['tx'])['vin'] + prev_utxos = ["{}:{}".format(vin['txid'], vin['vout']) for vin in vins if "{}:{}".format(vin['txid'], vin['vout']) in l1_utxos] + + # Check that we're waiting for lockin + l1.daemon.wait_for_log(' to DUALOPEND_AWAITING_LOCKIN') + + # Check that feerate info is correct + info_1 = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels']) + next_rate = "{}perkw".format(info_1['next_feerate'][:-5]) + + # Initiate an RBF + startweight = 42 + 172 # base weight, funding output + initpsbt = l1.rpc.utxopsbt(chan_amount, next_rate, startweight, + prev_utxos, reservedok=True, + min_witness_weight=110, + excess_as_change=True) + + # Do the bump + bump = l1.rpc.openchannel_bump(chan_id, chan_amount, initpsbt['psbt']) + + # We abort the channel mid-way throught the RBF + l1.rpc.openchannel_abort(chan_id) + + with pytest.raises(RpcError): + l1.rpc.openchannel_update(chan_id, bump['psbt']) + + # When the original open tx is mined, we should still arrive at + # NORMAL channel ops + bitcoind.generate_block(1) + sync_blockheight(bitcoind, [l1]) + l1.daemon.wait_for_log(' to CHANNELD_NORMAL') + + @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') @pytest.mark.openchannel('v2') def test_v2_rbf_liquidity_ad(node_factory, bitcoind, chainparams): From 17e8911e23d64a719008f7e51c37720b32771d46 Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 29 Jul 2023 20:41:26 -0500 Subject: [PATCH 3/3] dual-fund: keep track of aborted requests, seamlessly restart daemon Clean restart of daemon after a tx-abort is a nice way to work around the 'persistent' disconnect that we t-bast noticed. Changelog-Fixed: `dualopend`: Fix behavior for tx-aborts. No longer hangs, appropriately continues re-init of RBF requests without reconnction msg exchange. --- common/peer_failed.c | 6 +- common/peer_failed.h | 3 +- common/peer_status_wire.csv | 2 + common/read_peer_msg.c | 2 +- common/wire_error.c | 2 +- lightningd/dual_open_control.c | 159 +++++++++++++++++--- lightningd/dual_open_control.h | 3 +- lightningd/onchain_control.c | 1 + lightningd/opening_common.c | 1 + lightningd/opening_common.h | 1 + lightningd/peer_control.c | 11 +- lightningd/peer_control.h | 1 + lightningd/subd.c | 9 +- lightningd/subd.h | 4 +- lightningd/test/run-find_my_abspath.c | 2 +- lightningd/test/run-invoice-select-inchan.c | 3 +- lightningd/test/run-shuffle_fds.c | 2 +- openingd/dualopend.c | 13 +- openingd/dualopend_wire.csv | 1 + tests/test_opening.py | 2 - wallet/test/run-wallet.c | 3 +- 21 files changed, 181 insertions(+), 50 deletions(-) diff --git a/common/peer_failed.c b/common/peer_failed.c index 5281bf86da9f..900a8c5b8cb4 100644 --- a/common/peer_failed.c +++ b/common/peer_failed.c @@ -42,6 +42,7 @@ peer_failed(struct per_peer_state *pps, msg = towire_status_peer_error(NULL, channel_id, desc, warn, + false, msg); peer_billboard(true, desc); peer_fatal_continue(take(msg), pps); @@ -80,7 +81,8 @@ void peer_failed_err(struct per_peer_state *pps, void peer_failed_received_errmsg(struct per_peer_state *pps, const char *desc, const struct channel_id *channel_id, - bool warning) + bool warning, + bool abort_restart) { u8 *msg; @@ -94,7 +96,7 @@ void peer_failed_received_errmsg(struct per_peer_state *pps, warning = true; } msg = towire_status_peer_error(NULL, channel_id, desc, warning, - NULL); + abort_restart, NULL); peer_billboard(true, "Received %s", desc); peer_fatal_continue(take(msg), pps); } diff --git a/common/peer_failed.h b/common/peer_failed.h index db1ff26a2827..0cff74b2a5d9 100644 --- a/common/peer_failed.h +++ b/common/peer_failed.h @@ -37,7 +37,8 @@ void peer_failed_err(struct per_peer_state *pps, void peer_failed_received_errmsg(struct per_peer_state *pps, const char *desc, const struct channel_id *channel_id, - bool soft_error) + bool soft_error, + bool abort_restart) NORETURN; /* I/O error */ diff --git a/common/peer_status_wire.csv b/common/peer_status_wire.csv index 1fedb7ccc678..7ad49eb4201b 100644 --- a/common/peer_status_wire.csv +++ b/common/peer_status_wire.csv @@ -8,5 +8,7 @@ msgdata,status_peer_error,channel,channel_id, msgdata,status_peer_error,desc,wirestring, # Take a deep breath, then try reconnecting to the precious little snowflake. msgdata,status_peer_error,warning,bool, +# From an abort, no reconnect but restart daemon +msgdata,status_peer_error,abort_do_restart,bool, msgdata,status_peer_error,len,u16, msgdata,status_peer_error,error_for_them,u8,len diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index d61abfd53f56..830c51c53835 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -83,7 +83,7 @@ bool handle_peer_error(struct per_peer_state *pps, } /* We hang up when a warning is received. */ - peer_failed_received_errmsg(pps, err, channel_id, warning); + peer_failed_received_errmsg(pps, err, channel_id, warning, false); } return false; diff --git a/common/wire_error.c b/common/wire_error.c index 457dc72b738d..e4376924e8be 100644 --- a/common/wire_error.c +++ b/common/wire_error.c @@ -94,7 +94,7 @@ char *sanitize_error(const tal_t *ctx, const u8 *errmsg, else if (fromwire_warning(ctx, errmsg, channel_id, &data)) warning = true; else if (fromwire_tx_abort(ctx, errmsg, channel_id, &data)) - warning = false; + warning = true; else return tal_fmt(ctx, "Invalid ERROR message '%s'", tal_hex(ctx, errmsg)); diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index a6ccb741325e..91599d699f9f 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -1820,6 +1820,8 @@ static void handle_channel_locked(struct subd *dualopend, return; } + + void dualopen_tell_depth(struct subd *dualopend, struct channel *channel, const struct bitcoin_txid *txid, @@ -2312,6 +2314,33 @@ json_openchannel_abort(struct command *cmd, return command_still_pending(cmd); } +static char *restart_dualopend(const tal_t *ctx, const struct lightningd *ld, + struct channel *channel, bool from_abort) +{ + int fds[2]; + if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) { + log_broken(channel->log, + "Failed to create socketpair: %s", + strerror(errno)); + return tal_fmt(ctx, "Unable to create socket: %s", + strerror(errno)); + } + + if (!peer_restart_dualopend(channel->peer, + new_peer_fd(tmpctx, fds[0]), + channel, from_abort)) { + close(fds[1]); + return tal_fmt(ctx, "Peer not connected"); + } + subd_send_msg(ld->connectd, + take(towire_connectd_peer_connect_subd(NULL, + &channel->peer->id, + channel->peer->connectd_counter, + &channel->cid))); + subd_send_fd(ld->connectd, fds[1]); + return NULL; +} + static struct command_result * json_openchannel_bump(struct command *cmd, const char *buffer, @@ -2404,29 +2433,10 @@ json_openchannel_bump(struct command *cmd, /* It's possible that the last open failed/was aborted. * So now we restart the attempt! */ if (!channel->owner) { - int fds[2]; - if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) { - log_broken(channel->log, - "Failed to create socketpair: %s", - strerror(errno)); - return command_fail(cmd, FUNDING_PEER_NOT_CONNECTED, - "Unable to create socket: %s", - strerror(errno)); - } - - if (!peer_restart_dualopend(channel->peer, - new_peer_fd(tmpctx, fds[0]), - channel)) { - close(fds[1]); + char *err = restart_dualopend(cmd, cmd->ld, channel, false); + if (err) return command_fail(cmd, FUNDING_PEER_NOT_CONNECTED, - "Peer not connected."); - } - subd_send_msg(cmd->ld->connectd, - take(towire_connectd_peer_connect_subd(NULL, - &channel->peer->id, - channel->peer->connectd_counter, - &channel->cid))); - subd_send_fd(cmd->ld->connectd, fds[1]); + "%s", err); } if (channel->open_attempt) @@ -3574,6 +3584,103 @@ AUTODATA(json_command, &openchannel_signed_command); AUTODATA(json_command, &openchannel_bump_command); AUTODATA(json_command, &openchannel_abort_command); +void static dualopen_errmsg(struct channel *channel, + struct peer_fd *peer_fd, + const struct channel_id *channel_id UNUSED, + const char *desc, + bool warning, + bool aborted, + const u8 *err_for_them) +{ + /* Clean up any in-progress open attempts */ + channel_cleanup_commands(channel, desc); + + if (channel_unsaved(channel)) { + log_info(channel->log, "%s", "Unsaved peer failed." + " Deleting channel."); + delete_channel(channel); + return; + } + + /* No peer_fd means a subd crash or disconnection. */ + if (!peer_fd) { + /* If the channel is unsaved, we forget it */ + channel_fail_transient(channel, "%s: %s", + channel->owner->name, desc); + return; + } + + /* Do we have an error to send? */ + if (err_for_them && !channel->error && !warning) + channel->error = tal_dup_talarr(channel, u8, err_for_them); + + /* Other implementations chose to ignore errors early on. Not + * surprisingly, they now spew out spurious errors frequently, + * and we would close the channel on them. We now support warnings + * for this case. */ + if (warning || aborted) { + channel_fail_transient(channel, "%s %s: %s", + channel->owner->name, + warning ? "WARNING" : "ABORTED", + desc); + + if (aborted) { + char *err = restart_dualopend(tmpctx, + channel->peer->ld, + channel, true); + if (err) + log_broken(channel->log, + "Unable to restart dualopend" + " after abort: %s", err); + } + + return; + } + + /* BOLT #1: + * + * A sending node: + *... + * - when sending `error`: + * - MUST fail the channel(s) referred to by the error message. + * - MAY set `channel_id` to all zero to indicate all channels. + */ + /* FIXME: Close if it's an all-channels error sent or rcvd */ + + /* BOLT #1: + * + * A sending node: + *... + * - when sending `error`: + * - MUST fail the channel(s) referred to by the error message. + * - MAY set `channel_id` to all zero to indicate all channels. + *... + * The receiving node: + * - upon receiving `error`: + * - if `channel_id` is all zero: + * - MUST fail all channels with the sending node. + * - otherwise: + * - MUST fail the channel referred to by `channel_id`, if that channel is with the + * sending node. + */ + + /* FIXME: We don't close all channels */ + /* We should immediately forget the channel if we receive error during + * CHANNELD_AWAITING_LOCKIN if we are fundee. */ + if (!err_for_them && channel->opener == REMOTE + && channel->state == CHANNELD_AWAITING_LOCKIN) + channel_fail_forget(channel, "%s: %s ERROR %s", + channel->owner->name, + err_for_them ? "sent" : "received", desc); + else + channel_fail_permanent(channel, + err_for_them ? REASON_LOCAL : REASON_PROTOCOL, + "%s: %s ERROR %s", + channel->owner->name, + err_for_them ? "sent" : "received", desc); +} + + bool peer_start_dualopend(struct peer *peer, struct peer_fd *peer_fd, struct channel *channel) @@ -3596,7 +3703,7 @@ bool peer_start_dualopend(struct peer *peer, channel->log, true, dualopend_wire_name, dual_opend_msg, - channel_errmsg, + dualopen_errmsg, channel_set_billboard, take(&peer_fd->fd), take(&hsmfd), NULL); @@ -3641,7 +3748,8 @@ bool peer_start_dualopend(struct peer *peer, bool peer_restart_dualopend(struct peer *peer, struct peer_fd *peer_fd, - struct channel *channel) + struct channel *channel, + bool from_abort) { u32 max_to_self_delay, blockheight; struct amount_msat min_effective_htlc_capacity; @@ -3667,7 +3775,7 @@ bool peer_restart_dualopend(struct peer *peer, channel->log, true, dualopend_wire_name, dual_opend_msg, - channel_errmsg, + dualopen_errmsg, channel_set_billboard, take(&peer_fd->fd), take(&hsmfd), NULL)); @@ -3706,6 +3814,7 @@ bool peer_restart_dualopend(struct peer *peer, msg = towire_dualopend_reinit(NULL, chainparams, + from_abort, peer->ld->our_features, peer->their_features, &channel->our_config, diff --git a/lightningd/dual_open_control.h b/lightningd/dual_open_control.h index 63c51bc3f3df..e3e0e92677fd 100644 --- a/lightningd/dual_open_control.h +++ b/lightningd/dual_open_control.h @@ -11,7 +11,8 @@ bool peer_start_dualopend(struct peer *peer, struct peer_fd *peer_fd, bool peer_restart_dualopend(struct peer *peer, struct peer_fd *peer_fd, - struct channel *channel); + struct channel *channel, + bool from_abort); void dualopen_tell_depth(struct subd *dualopend, struct channel *channel, diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index ea531c0d23fd..edb143cd1d10 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -1496,6 +1496,7 @@ static void onchain_error(struct channel *channel, const struct channel_id *channel_id UNUSED, const char *desc, bool warning UNUSED, + bool aborted UNUSED, const u8 *err_for_them UNUSED) { channel_set_owner(channel, NULL); diff --git a/lightningd/opening_common.c b/lightningd/opening_common.c index e1dbf1b30780..6842c00eb0f8 100644 --- a/lightningd/opening_common.c +++ b/lightningd/opening_common.c @@ -96,6 +96,7 @@ void opend_channel_errmsg(struct uncommitted_channel *uc, const struct channel_id *channel_id UNUSED, const char *desc, bool warning UNUSED, + bool aborted UNUSED, const u8 *err_for_them UNUSED) { /* Close fds, if any. */ diff --git a/lightningd/opening_common.h b/lightningd/opening_common.h index 7fed2721b29e..4ca69409f33b 100644 --- a/lightningd/opening_common.h +++ b/lightningd/opening_common.h @@ -112,6 +112,7 @@ void opend_channel_errmsg(struct uncommitted_channel *uc, const struct channel_id *channel_id UNUSED, const char *desc, bool warning UNUSED, + bool aborted UNUSED, const u8 *err_for_them UNUSED); void opend_channel_set_billboard(struct uncommitted_channel *uc, diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index e367ceeee9cd..229cc3682936 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -375,6 +375,7 @@ void channel_errmsg(struct channel *channel, const struct channel_id *channel_id UNUSED, const char *desc, bool warning, + bool aborted UNUSED, const u8 *err_for_them) { /* Clean up any in-progress open attempts */ @@ -404,8 +405,10 @@ void channel_errmsg(struct channel *channel, * and we would close the channel on them. We now support warnings * for this case. */ if (warning) { - channel_fail_transient(channel, "%s WARNING: %s", - channel->owner->name, desc); + channel_fail_transient(channel, "%s%s: %s", + channel->owner->name, + warning ? " WARNING" : " (aborted)", + desc); return; } @@ -1160,7 +1163,7 @@ static void connect_activate_subd(struct lightningd *ld, struct channel *channel } if (peer_restart_dualopend(channel->peer, new_peer_fd(tmpctx, fds[0]), - channel)) + channel, false)) goto tell_connectd; close(fds[1]); return; @@ -1529,7 +1532,7 @@ void peer_spoke(struct lightningd *ld, const u8 *msg) "Trouble in paradise?"); goto send_error; } - if (peer_restart_dualopend(peer, new_peer_fd(tmpctx, fds[0]), channel)) + if (peer_restart_dualopend(peer, new_peer_fd(tmpctx, fds[0]), channel, false)) goto tell_connectd; /* FIXME: Send informative error? */ close(fds[1]); diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index 9c69faac302f..70eaf6bf7527 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -97,6 +97,7 @@ void channel_errmsg(struct channel *channel, const struct channel_id *channel_id, const char *desc, bool warning, + bool aborted, const u8 *err_for_them); u8 *p2wpkh_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx); diff --git a/lightningd/subd.c b/lightningd/subd.c index 9e785e90b9cf..5ae43b1110f1 100644 --- a/lightningd/subd.c +++ b/lightningd/subd.c @@ -425,17 +425,18 @@ static bool handle_peer_error(struct subd *sd, const u8 *msg, int fds[1]) struct peer_fd *peer_fd; u8 *err_for_them; bool warning; + bool aborted; if (!fromwire_status_peer_error(msg, msg, &channel_id, &desc, &warning, - &err_for_them)) + &aborted, &err_for_them)) return false; peer_fd = new_peer_fd_arr(msg, fds); /* Don't free sd; we may be about to free channel. */ sd->channel = NULL; - sd->errcb(channel, peer_fd, &channel_id, desc, warning, err_for_them); + sd->errcb(channel, peer_fd, &channel_id, desc, warning, aborted, err_for_them); return true; } @@ -648,7 +649,7 @@ static void destroy_subd(struct subd *sd) sd->errcb(channel, NULL, NULL, tal_fmt(sd, "Owning subdaemon %s died (%i)", sd->name, status), - false, NULL); + false, false, NULL); if (!outer_transaction) db_commit_transaction(db); } @@ -706,6 +707,7 @@ static struct subd *new_subd(const tal_t *ctx, const struct channel_id *channel_id, const char *desc, bool warning, + bool aborted, const u8 *err_for_them), void (*billboardcb)(void *channel, bool perm, @@ -816,6 +818,7 @@ struct subd *new_channel_subd_(const tal_t *ctx, const struct channel_id *channel_id, const char *desc, bool warning, + bool aborted, const u8 *err_for_them), void (*billboardcb)(void *channel, bool perm, const char *happenings), diff --git a/lightningd/subd.h b/lightningd/subd.h index 1dd218c2f245..b6528f4caf92 100644 --- a/lightningd/subd.h +++ b/lightningd/subd.h @@ -54,6 +54,7 @@ struct subd { const struct channel_id *channel_id, const char *desc, bool warning, + bool aborted, const u8 *err_for_them); /* Callback to display information for listpeers RPC */ @@ -136,6 +137,7 @@ struct subd *new_channel_subd_(const tal_t *ctx, const struct channel_id *channel_id, const char *desc, bool warning, + bool aborted, const u8 *err_for_them), void (*billboardcb)(void *channel, bool perm, const char *happenings), @@ -151,7 +153,7 @@ struct subd *new_channel_subd_(const tal_t *ctx, (channel), \ struct peer_fd *, \ const struct channel_id *, \ - const char *, bool, const u8 *), \ + const char *, bool, bool, const u8 *), \ typesafe_cb_postargs(void, void *, (billboardcb), \ (channel), bool, \ const char *), \ diff --git a/lightningd/test/run-find_my_abspath.c b/lightningd/test/run-find_my_abspath.c index 6f01a9e83faf..171997f864ab 100644 --- a/lightningd/test/run-find_my_abspath.c +++ b/lightningd/test/run-find_my_abspath.c @@ -84,7 +84,7 @@ bool fromwire_status_fail(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, enu bool fromwire_status_peer_billboard(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, bool *perm UNNEEDED, wirestring **happenings UNNEEDED) { fprintf(stderr, "fromwire_status_peer_billboard called!\n"); abort(); } /* Generated stub for fromwire_status_peer_error */ -bool fromwire_status_peer_error(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct channel_id *channel UNNEEDED, wirestring **desc UNNEEDED, bool *warning UNNEEDED, u8 **error_for_them UNNEEDED) +bool fromwire_status_peer_error(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct channel_id *channel UNNEEDED, wirestring **desc UNNEEDED, bool *warning UNNEEDED, bool *abort_do_restart UNNEEDED, u8 **error_for_them UNNEEDED) { fprintf(stderr, "fromwire_status_peer_error called!\n"); abort(); } /* Generated stub for fromwire_status_version */ bool fromwire_status_version(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, wirestring **version UNNEEDED) diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 3fb2edd137ba..3af8a13c6f52 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -796,7 +796,8 @@ struct channel *peer_any_active_channel(struct peer *peer UNNEEDED, bool *others /* Generated stub for peer_restart_dualopend */ bool peer_restart_dualopend(struct peer *peer UNNEEDED, struct peer_fd *peer_fd UNNEEDED, - struct channel *channel UNNEEDED) + struct channel *channel UNNEEDED, + bool from_abort UNNEEDED) { fprintf(stderr, "peer_restart_dualopend called!\n"); abort(); } /* Generated stub for peer_start_channeld */ bool peer_start_channeld(struct channel *channel UNNEEDED, diff --git a/lightningd/test/run-shuffle_fds.c b/lightningd/test/run-shuffle_fds.c index 9e5e5b7453e2..f7c4a0a67989 100644 --- a/lightningd/test/run-shuffle_fds.c +++ b/lightningd/test/run-shuffle_fds.c @@ -70,7 +70,7 @@ bool fromwire_status_fail(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, enu bool fromwire_status_peer_billboard(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, bool *perm UNNEEDED, wirestring **happenings UNNEEDED) { fprintf(stderr, "fromwire_status_peer_billboard called!\n"); abort(); } /* Generated stub for fromwire_status_peer_error */ -bool fromwire_status_peer_error(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct channel_id *channel UNNEEDED, wirestring **desc UNNEEDED, bool *warning UNNEEDED, u8 **error_for_them UNNEEDED) +bool fromwire_status_peer_error(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct channel_id *channel UNNEEDED, wirestring **desc UNNEEDED, bool *warning UNNEEDED, bool *abort_do_restart UNNEEDED, u8 **error_for_them UNNEEDED) { fprintf(stderr, "fromwire_status_peer_error called!\n"); abort(); } /* Generated stub for fromwire_status_version */ bool fromwire_status_version(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, wirestring **version UNNEEDED) diff --git a/openingd/dualopend.c b/openingd/dualopend.c index 17b0453665c5..1fa6a3a87c3c 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -326,13 +326,13 @@ static bool shutdown_complete(const struct state *state) } /* They failed the open with us */ -static void negotiation_aborted(struct state *state, const char *why) +static void negotiation_aborted(struct state *state, const char *why, bool aborted) { status_debug("aborted opening negotiation: %s", why); /* Tell master that funding failed. */ peer_failed_received_errmsg(state->pps, why, - &state->channel_id, true); + &state->channel_id, true, aborted); } /* Softer version of 'warning' (we don't disconnect) @@ -1269,7 +1269,7 @@ static void handle_tx_abort(struct state *state, u8 *msg) } else desc = state->aborted_err; - negotiation_aborted(state, desc); + negotiation_aborted(state, desc, true); } static u8 *handle_channel_ready(struct state *state, u8 *msg) @@ -1352,7 +1352,7 @@ static u8 *opening_negotiate_msg(const tal_t *ctx, struct state *state) } negotiation_aborted(state, tal_fmt(tmpctx, "They sent %s", - err)); + err), false); /* Return NULL so caller knows to stop negotiating. */ return NULL; } @@ -4268,6 +4268,7 @@ int main(int argc, char *argv[]) u8 *msg; struct amount_sat total_funding, *requested_lease; struct amount_msat our_msat; + bool from_abort; subdaemon_setup(argc, argv); @@ -4297,6 +4298,7 @@ int main(int argc, char *argv[]) /*~ Initially we're not associated with a channel, but * handle_peer_gossip_or_error compares this. */ memset(&state->channel_id, 0, sizeof(state->channel_id)); + from_abort = false; state->channel = NULL; state->tx_state->remote_funding_sigs_rcvd = false; @@ -4317,6 +4319,7 @@ int main(int argc, char *argv[]) state->requested_lease = NULL; } else if (fromwire_dualopend_reinit(state, msg, &chainparams, + &from_abort, &state->our_features, &state->their_features, &state->tx_state->localconf, @@ -4431,7 +4434,7 @@ int main(int argc, char *argv[]) pollfd[1].events = POLLIN; /* Do reconnect, if need be */ - if (state->channel) { + if (state->channel && !from_abort) { do_reconnect_dance(state); state->reconnected = true; } diff --git a/openingd/dualopend_wire.csv b/openingd/dualopend_wire.csv index 8320000e4a41..c2c8217abf6c 100644 --- a/openingd/dualopend_wire.csv +++ b/openingd/dualopend_wire.csv @@ -33,6 +33,7 @@ msgdata,dualopend_init,require_confirmed_inputs,bool, # master-dualopend: peer has reconnected msgtype,dualopend_reinit,7001 msgdata,dualopend_reinit,chainparams,chainparams, +msgdata,dualopend_reinit,from_abort,bool, msgdata,dualopend_reinit,our_feature_set,feature_set, msgdata,dualopend_reinit,their_init_features_len,u16, msgdata,dualopend_reinit,their_init_features,u8,their_init_features_len diff --git a/tests/test_opening.py b/tests/test_opening.py index e7dfc5955486..590f2fe2f127 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -380,7 +380,6 @@ def test_v2_rbf_single(node_factory, bitcoind, chainparams): @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') @pytest.mark.openchannel('v2') -@pytest.mark.xfail def test_v2_rbf_abort_retry(node_factory, bitcoind, chainparams): l1, l2 = node_factory.get_nodes(2, opts={'wumbo': None, 'allow_warning': True}) @@ -463,7 +462,6 @@ def test_v2_rbf_abort_retry(node_factory, bitcoind, chainparams): @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') -@pytest.mark.xfail @pytest.mark.openchannel('v2') def test_v2_rbf_abort_channel_opens(node_factory, bitcoind, chainparams): l1, l2 = node_factory.get_nodes(2, opts={'wumbo': None, diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 68693ecf936b..59586848a1ac 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -629,7 +629,8 @@ void payment_succeeded(struct lightningd *ld UNNEEDED, /* Generated stub for peer_restart_dualopend */ bool peer_restart_dualopend(struct peer *peer UNNEEDED, struct peer_fd *peer_fd UNNEEDED, - struct channel *channel UNNEEDED) + struct channel *channel UNNEEDED, + bool from_abort UNNEEDED) { fprintf(stderr, "peer_restart_dualopend called!\n"); abort(); } /* Generated stub for peer_start_channeld */ bool peer_start_channeld(struct channel *channel UNNEEDED,