From 382264e207d131939e9d5c685e34dd86fb25f089 Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 20 Apr 2021 16:42:52 -0500 Subject: [PATCH] dualopend: don't use final channel_id for accepter_start2 The other side doesn't know it until *after* it parses this msg. We add a quick hack to still allow old nodes to work (for now!). This also fixes a bug (spotted by @niftynei) where any errors we sent before accepter_start2 would have the new (unknowable!) channel_id rather than the temp one. Authored-by: Rusty Russell --- openingd/dualopend.c | 55 ++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/openingd/dualopend.c b/openingd/dualopend.c index ad8d5420e..1030316d9 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -1896,8 +1896,8 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) { struct bitcoin_blkid chain_hash; struct tlv_opening_tlvs *open_tlv; + struct channel_id cid, full_cid; char *err_reason; - struct channel_id tmp_chan_id; u8 *msg; struct amount_sat total; enum dualopend_wire msg_type; @@ -1907,7 +1907,7 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) open_tlv = tlv_opening_tlvs_new(tmpctx); if (!fromwire_open_channel2(oc2_msg, &chain_hash, - &state->channel_id, /* Temporary! */ + &cid, &tx_state->feerate_per_kw_funding, &state->feerate_per_kw_commitment, &tx_state->opener_funding, @@ -1939,21 +1939,15 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) * `open_channel2`), a temporary `channel_id` should be found * by using a zeroed out basepoint for the unknown peer. */ - derive_tmp_channel_id(&tmp_chan_id, + derive_tmp_channel_id(&state->channel_id, /* Temporary! */ &state->their_points.revocation); - if (!channel_id_eq(&state->channel_id, &tmp_chan_id)) + if (!channel_id_eq(&state->channel_id, &cid)) negotiation_failed(state, "open_channel2 channel_id incorrect." " Expected %s, received %s", type_to_string(tmpctx, struct channel_id, - &tmp_chan_id), + &state->channel_id), type_to_string(tmpctx, struct channel_id, - &state->channel_id)); - - /* Everything's ok. Let's figure out the actual channel_id now */ - derive_channel_id_v2(&state->channel_id, - &state->our_points.revocation, - &state->their_points.revocation); - + &cid)); /* Save feerate on the state as well */ state->feerate_per_kw_funding = tx_state->feerate_per_kw_funding; @@ -1990,8 +1984,12 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) return; } + /* We send the 'real' channel id over to lightningd */ + derive_channel_id_v2(&full_cid, + &state->our_points.revocation, + &state->their_points.revocation); msg = towire_dualopend_got_offer(NULL, - &state->channel_id, + &full_cid, tx_state->opener_funding, tx_state->remoteconf.dust_limit, tx_state->remoteconf.max_htlc_value_in_flight, @@ -2010,7 +2008,6 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) if ((msg_type = fromwire_peektype(msg)) == WIRE_DUALOPEND_FAIL) { if (!fromwire_dualopend_fail(msg, msg, &err_reason)) master_badmsg(msg_type, msg); - open_err_warn(state, "%s", err_reason); return; } @@ -2105,6 +2102,11 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) &state->first_per_commitment_point[LOCAL], a_tlv); + /* Everything's ok. Let's figure out the actual channel_id now */ + derive_channel_id_v2(&state->channel_id, + &state->our_points.revocation, + &state->their_points.revocation); + sync_crypto_write(state->pps, msg); peer_billboard(false, "channel open: accept sent, waiting for reply"); @@ -2479,6 +2481,23 @@ static void opener_start(struct state *state, u8 *msg) open_err_fatal(state, "Parsing accept_channel2 %s", tal_hex(msg, msg)); + if (!channel_id_eq(&cid, &state->channel_id)) { + struct channel_id future_chan_id; + /* FIXME: v0.10.0 actually replied with the complete channel id here, + * so we need to accept it for now */ + derive_channel_id_v2(&future_chan_id, + &state->our_points.revocation, + &state->their_points.revocation); + if (!channel_id_eq(&cid, &future_chan_id)) { + peer_failed_err(state->pps, &cid, + "accept_channel2 ids don't match: " + "expected %s, got %s", + type_to_string(msg, struct channel_id, + &state->channel_id), + type_to_string(msg, struct channel_id, &cid)); + } + } + if (a_tlv->option_upfront_shutdown_script) { state->upfront_shutdown_script[REMOTE] = tal_steal(state, @@ -2492,14 +2511,6 @@ static void opener_start(struct state *state, u8 *msg) &state->our_points.revocation, &state->their_points.revocation); - if (!channel_id_eq(&cid, &state->channel_id)) - peer_failed_err(state->pps, &cid, - "accept_channel2 ids don't match: " - "expected %s, got %s", - type_to_string(msg, struct channel_id, - &state->channel_id), - type_to_string(msg, struct channel_id, &cid)); - /* Check that total funding doesn't overflow */ if (!amount_sat_add(&total, tx_state->opener_funding, tx_state->accepter_funding))