Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ static void check_mutual_splice_locked(struct peer *peer)
fmt_channel(tmpctx, peer->channel));

error = channel_update_funding(peer->channel, &inflight->outpoint,
inflight->funding_tx_index,
inflight->amnt,
inflight->splice_amnt);
if (error)
Expand Down Expand Up @@ -4362,6 +4363,7 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg)
&peer->splicing->remote_funding_pubkey,
&outpoint.txid,
outpoint.n,
peer->channel->funding_tx_index + 1,
funding_feerate_perkw,
both_amount,
peer->splicing->accepter_relative,
Expand All @@ -4379,6 +4381,8 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg)
&new_inflight->outpoint.txid, NULL);
new_inflight->remote_funding = peer->splicing->remote_funding_pubkey;
new_inflight->outpoint = outpoint;
/* A splice's funding tx is the parent funding's index + 1. */
new_inflight->funding_tx_index = peer->channel->funding_tx_index + 1;
new_inflight->amnt = both_amount;
new_inflight->psbt = clone_psbt(new_inflight, ictx->current_psbt);
new_inflight->splice_amnt = peer->splicing->accepter_relative;
Expand Down Expand Up @@ -4657,6 +4661,7 @@ static void splice_initiator_user_finalized(struct peer *peer)
&peer->splicing->remote_funding_pubkey,
&current_psbt_txid,
chan_output_index,
peer->channel->funding_tx_index + 1,
peer->splicing->feerate_per_kw,
amount_sat(new_chan_output->amount),
peer->splicing->opener_relative,
Expand All @@ -4674,6 +4679,8 @@ static void splice_initiator_user_finalized(struct peer *peer)
NULL);
new_inflight->remote_funding = peer->splicing->remote_funding_pubkey;
new_inflight->outpoint.n = chan_output_index;
/* A splice's funding tx is the parent funding's index + 1. */
new_inflight->funding_tx_index = peer->channel->funding_tx_index + 1;
new_inflight->amnt = amount_sat(new_chan_output->amount);
new_inflight->splice_amnt = peer->splicing->opener_relative;
new_inflight->last_tx = NULL;
Expand Down Expand Up @@ -5638,6 +5645,23 @@ static bool capture_premature_msg(const u8 ***shit_lnd_says, const u8 *msg)
return true;
}

/* Returns the funding_tx_index of the funding tx with this txid: the current
* channel funding, or a pending splice inflight. Returns 0 (the original
* funding) if unknown.
*/
static u32 funding_tx_index_for_txid(const struct peer *peer,
const struct bitcoin_txid *txid)
{
if (bitcoin_txid_eq(txid, &peer->channel->funding.txid))
return peer->channel->funding_tx_index;
for (size_t i = 0; i < tal_count(peer->splice_state->inflights); i++) {
const struct inflight *inf = peer->splice_state->inflights[i];
if (bitcoin_txid_eq(txid, &inf->outpoint.txid))
return inf->funding_tx_index;
}
return 0;
}

static void peer_reconnect(struct peer *peer,
const struct secret *last_remote_per_commit_secret)
{
Expand Down Expand Up @@ -5967,9 +5991,8 @@ static void peer_reconnect(struct peer *peer,
|| remote_next_funding
|| (recv_tlvs
&& recv_tlvs->my_current_funding_locked
&& !bitcoin_txid_eq(
&recv_tlvs->my_current_funding_locked->my_current_funding_locked_txid,
&peer->channel->funding.txid));
&& funding_tx_index_for_txid(peer,
&recv_tlvs->my_current_funding_locked->my_current_funding_locked_txid) > 0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct style is to have the BOLT reference at the code where it is used and the code needs to match the spec language as closely as possible. This is for many reasons.

I understand moving the check into the is_splice_active variable may feel like a clean up, but being clean to the spec is more important here.

Let's drop this variable, put the check back where it was on the channel_ready check, and use a correct BOLT reference.

The bolt reference should read something like:

	/* BOLT #2:
	 *  - if next_commitment_number is 1 in both the channel_reestablish it sent and received, and none of those channel_reestablish messages contain my_current_funding_locked or next_funding for a splice transaction:
	 *   - MUST retransmit channel_ready.
	 * - otherwise:
	 *    - MUST NOT retransmit channel_ready, but MAY send channel_ready with a different short_channel_id alias field.
	 * /

Using the Bolt reference this way makes CI enforce the bolt reference is correct and serves as a reminder to easily scan the code directly beneath it to verify we are following the spec.

This ensures we're keeping up with the spec as it changes, makes audits significantly easier, and even helps us notice when the spec itself is wrong so we can update the spec.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, adding a comment here explaining something like "tx_index of 1 or more means it was from a splice." could be useful.


/* BOLT #2:
*
Expand Down Expand Up @@ -6825,6 +6848,7 @@ static void init_channel(struct peer *peer)
{
struct basepoints points[NUM_SIDES];
struct amount_sat funding_sats;
u32 funding_tx_index;
struct amount_msat local_msat;
struct pubkey funding_pubkey[NUM_SIDES];
struct channel_config conf[NUM_SIDES];
Expand Down Expand Up @@ -6854,6 +6878,7 @@ static void init_channel(struct peer *peer)
&peer->channel_id,
&funding,
&funding_sats,
&funding_tx_index,
&minimum_depth,
&peer->our_blockheight,
&blockheight_states,
Expand Down Expand Up @@ -6968,6 +6993,7 @@ static void init_channel(struct peer *peer)

peer->channel = new_full_channel(peer, &peer->channel_id,
&funding,
funding_tx_index,
minimum_depth,
take(blockheight_states),
lease_expiry,
Expand Down
2 changes: 2 additions & 0 deletions channeld/channeld_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ msgdata,channeld_init,hsm_capabilities,u32,num_hsm_capabilities
msgdata,channeld_init,channel_id,channel_id,
msgdata,channeld_init,funding,bitcoin_outpoint,
msgdata,channeld_init,funding_satoshi,amount_sat,
msgdata,channeld_init,funding_tx_index,u32,
msgdata,channeld_init,minimum_depth,u32,
msgdata,channeld_init,our_blockheight,u32,
msgdata,channeld_init,blockheight_states,height_states,
Expand Down Expand Up @@ -261,6 +262,7 @@ msgtype,channeld_add_inflight,7216
msgdata,channeld_add_inflight,remote_funding,pubkey,
msgdata,channeld_add_inflight,tx_id,bitcoin_txid,
msgdata,channeld_add_inflight,tx_outnum,u32,
msgdata,channeld_add_inflight,funding_tx_index,u32,
msgdata,channeld_add_inflight,feerate,u32,
msgdata,channeld_add_inflight,satoshis,amount_sat,
msgdata,channeld_add_inflight,splice_amount,s64,
Expand Down
2 changes: 2 additions & 0 deletions channeld/full_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ static bool balance_ok(const struct balance *balance,
struct channel *new_full_channel(const tal_t *ctx,
const struct channel_id *cid,
const struct bitcoin_outpoint *funding,
u32 funding_tx_index,
u32 minimum_depth,
const struct height_states *blockheight_states,
u32 lease_expiry,
Expand All @@ -93,6 +94,7 @@ struct channel *new_full_channel(const tal_t *ctx,
struct channel *channel = new_initial_channel(ctx,
cid,
funding,
funding_tx_index,
minimum_depth,
blockheight_states,
lease_expiry,
Expand Down
1 change: 1 addition & 0 deletions channeld/full_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ struct existing_htlc;
struct channel *new_full_channel(const tal_t *ctx,
const struct channel_id *cid,
const struct bitcoin_outpoint *funding,
u32 funding_tx_index,
u32 minimum_depth,
const struct height_states *blockheight_states,
u32 lease_expiry,
Expand Down
2 changes: 2 additions & 0 deletions channeld/inflight.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ struct inflight *fromwire_inflight(const tal_t *ctx, const u8 **cursor, size_t *
struct inflight *inflight = tal(ctx, struct inflight);

fromwire_bitcoin_outpoint(cursor, max, &inflight->outpoint);
inflight->funding_tx_index = fromwire_u32(cursor, max);
fromwire_pubkey(cursor, max, &inflight->remote_funding);
inflight->amnt = fromwire_amount_sat(cursor, max);
inflight->remote_tx_sigs = fromwire_bool(cursor, max);
Expand Down Expand Up @@ -41,6 +42,7 @@ struct inflight *fromwire_inflight(const tal_t *ctx, const u8 **cursor, size_t *
void towire_inflight(u8 **pptr, const struct inflight *inflight)
{
towire_bitcoin_outpoint(pptr, &inflight->outpoint);
towire_u32(pptr, inflight->funding_tx_index);
towire_pubkey(pptr, &inflight->remote_funding);
towire_amount_sat(pptr, inflight->amnt);
towire_bool(pptr, inflight->remote_tx_sigs);
Expand Down
3 changes: 3 additions & 0 deletions channeld/inflight.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
struct inflight {
/* The new channel outpoint */
struct bitcoin_outpoint outpoint;
/* Which funding tx this is: 0 for the original funding (incl. RBF
* attempts), incrementing by 1 for each splice. */
u32 funding_tx_index;
struct pubkey remote_funding;
struct amount_sat amnt;
bool remote_tx_sigs;
Expand Down
4 changes: 2 additions & 2 deletions channeld/test/run-full_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ int main(int argc, const char *argv[])
feerate_per_kw[LOCAL] = feerate_per_kw[REMOTE] = 15000;
derive_channel_id(&cid, &funding);
lchannel = new_full_channel(tmpctx, &cid,
&funding, 0,
&funding, 0, 0,
take(new_height_states(NULL, LOCAL, &blockheight)),
0, /* No channel lease */
funding_amount, to_local,
Expand All @@ -505,7 +505,7 @@ int main(int argc, const char *argv[])
&remote_funding_pubkey,
take(channel_type_static_remotekey(NULL)), false, LOCAL);
rchannel = new_full_channel(tmpctx, &cid,
&funding, 0,
&funding, 0, 0,
take(new_height_states(NULL, REMOTE, &blockheight)),
0, /* No channel lease */
funding_amount, to_remote,
Expand Down
4 changes: 4 additions & 0 deletions common/initial_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
struct channel *new_initial_channel(const tal_t *ctx,
const struct channel_id *cid,
const struct bitcoin_outpoint *funding,
u32 funding_tx_index,
u32 minimum_depth,
const struct height_states *height_states TAKES,
u32 lease_expiry,
Expand Down Expand Up @@ -46,6 +47,7 @@ struct channel *new_initial_channel(const tal_t *ctx,

channel->cid = *cid;
channel->funding = *funding;
channel->funding_tx_index = funding_tx_index;
channel->funding_sats = funding_sats;
channel->minimum_depth = minimum_depth;
channel->lease_expiry = lease_expiry;
Expand Down Expand Up @@ -156,6 +158,7 @@ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx,

const char *channel_update_funding(struct channel *channel,
const struct bitcoin_outpoint *funding,
u32 funding_tx_index,
struct amount_sat funding_sats,
s64 splice_amnt)
{
Expand All @@ -164,6 +167,7 @@ const char *channel_update_funding(struct channel *channel,

channel->funding = *funding;
channel->funding_sats = funding_sats;
channel->funding_tx_index = funding_tx_index;

if (splice_amnt * 1000 + channel->view[LOCAL].owed[LOCAL].millisatoshis < 0) /* Raw: splicing */
return tal_fmt(tmpctx, "Channel funding update would make local"
Expand Down
7 changes: 7 additions & 0 deletions common/initial_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ struct channel {
/* Funding txid and output. */
struct bitcoin_outpoint funding;

/* Which funding tx this is: 0 for the original funding (incl. RBF
* attempts), incrementing by 1 for each splice. */
u32 funding_tx_index;

/* Keys used to spend funding tx. */
struct pubkey funding_pubkey[NUM_SIDES];

Expand Down Expand Up @@ -88,6 +92,7 @@ struct channel {
* @ctx: tal context to allocate return value from.
* @cid: The channel's id.
* @funding: The commitment transaction id/outnum
* @funding_tx_index: 0 for the original funding, +1 for each splice.
* @minimum_depth: The minimum confirmations needed for funding transaction.
* @height_states: The blockheight update states.
* @lease_expiry: Block the lease expires.
Expand All @@ -109,6 +114,7 @@ struct channel {
struct channel *new_initial_channel(const tal_t *ctx,
const struct channel_id *cid,
const struct bitcoin_outpoint *funding,
u32 funding_tx_index,
u32 minimum_depth,
const struct height_states *height_states TAKES,
u32 lease_expiry,
Expand Down Expand Up @@ -154,6 +160,7 @@ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx,
*/
const char *channel_update_funding(struct channel *channel,
const struct bitcoin_outpoint *funding,
u32 funding_tx_index,
struct amount_sat funding_sats,
s64 splice_amnt);

Expand Down
2 changes: 1 addition & 1 deletion devtools/mkcommit.c
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ int main(int argc, char *argv[])

channel = new_full_channel(NULL,
&cid,
&funding, 1,
&funding, 0, 1,
take(new_height_states(NULL, fee_payer,
&blockheight)),
0, /* Defaults to no lease */
Expand Down
5 changes: 4 additions & 1 deletion lightningd/channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ struct channel_inflight *
new_inflight(struct channel *channel,
struct pubkey *remote_funding,
const struct bitcoin_outpoint *funding_outpoint,
u32 funding_tx_index,
u32 funding_feerate,
struct amount_sat total_funds,
struct amount_sat our_funds,
Expand Down Expand Up @@ -199,6 +200,7 @@ new_inflight(struct channel *channel,
funding->splice_remote_funding = tal_steal(funding, remote_funding);

inflight->funding = funding;
inflight->funding_tx_index = funding_tx_index;
inflight->channel = channel;
inflight->remote_tx_sigs = false;
inflight->funding_psbt = tal_steal(inflight, psbt);
Expand Down Expand Up @@ -506,6 +508,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
u64 next_index_remote,
u64 next_htlc_id,
const struct bitcoin_outpoint *funding,
u32 funding_tx_index,
struct amount_sat funding_sats,
struct amount_msat push,
struct amount_sat our_funds,
Expand Down Expand Up @@ -612,6 +615,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
channel->next_index[REMOTE] = next_index_remote;
channel->next_htlc_id = next_htlc_id;
channel->funding = *funding;
channel->funding_tx_index = funding_tx_index;
channel->funding_sats = funding_sats;
channel->funding_spend_watch = NULL;
channel->push = push;
Expand Down Expand Up @@ -1338,4 +1342,3 @@ const u8 *channel_update_for_error(const tal_t *ctx,

return channel_gossip_update_for_error(ctx, channel);
}

8 changes: 8 additions & 0 deletions lightningd/channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ struct channel_inflight {

/* Funding info */
const struct funding_info *funding;
/* Which funding tx this is: 0 for the original funding (incl. RBF
* attempts), incrementing by 1 for each splice. */
u32 funding_tx_index;
struct wally_psbt *funding_psbt;
bool remote_tx_sigs;
bool tx_broadcast;
Expand Down Expand Up @@ -203,6 +206,9 @@ struct channel {

/* Funding outpoint and amount */
struct bitcoin_outpoint funding;
/* Which funding tx this is: 0 for the original funding (incl. RBF
* attempts), incrementing by 1 for each splice. */
u32 funding_tx_index;
struct amount_sat funding_sats;

/* Watch we have on funding output. */
Expand Down Expand Up @@ -394,6 +400,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
u64 next_index_remote,
u64 next_htlc_id,
const struct bitcoin_outpoint *funding,
u32 funding_tx_index,
struct amount_sat funding_sats,
struct amount_msat push,
struct amount_sat our_funds,
Expand Down Expand Up @@ -458,6 +465,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
struct channel_inflight *new_inflight(struct channel *channel,
struct pubkey *remote_funding STEALS,
const struct bitcoin_outpoint *funding_outpoint,
u32 funding_tx_index,
u32 funding_feerate,
struct amount_sat funding_sat,
struct amount_sat our_funds,
Expand Down
5 changes: 5 additions & 0 deletions lightningd/channel_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,7 @@ static void handle_add_inflight(struct lightningd *ld,
{
struct pubkey *remote_funding = tal(tmpctx, struct pubkey);
struct bitcoin_outpoint outpoint;
u32 funding_tx_index;
u32 feerate;
struct amount_sat satoshis;
s64 splice_amnt;
Expand All @@ -903,6 +904,7 @@ static void handle_add_inflight(struct lightningd *ld,
remote_funding,
&outpoint.txid,
&outpoint.n,
&funding_tx_index,
&feerate,
&satoshis,
&splice_amnt,
Expand All @@ -919,6 +921,7 @@ static void handle_add_inflight(struct lightningd *ld,
inflight = new_inflight(channel,
remote_funding,
&outpoint,
funding_tx_index,
feerate,
satoshis,
channel->our_funds,
Expand Down Expand Up @@ -1883,6 +1886,7 @@ bool peer_start_channeld(struct channel *channel,

infcopy->remote_funding = *inflight->funding->splice_remote_funding;
infcopy->outpoint = inflight->funding->outpoint;
infcopy->funding_tx_index = inflight->funding_tx_index;
infcopy->amnt = inflight->funding->total_funds;
infcopy->remote_tx_sigs = inflight->remote_tx_sigs;
infcopy->splice_amnt = inflight->funding->splice_amnt;
Expand Down Expand Up @@ -1910,6 +1914,7 @@ bool peer_start_channeld(struct channel *channel,
&channel->cid,
&channel->funding,
channel->funding_sats,
channel->funding_tx_index,
channel->minimum_depth,
curr_blockheight,
channel->blockheight_states,
Expand Down
2 changes: 2 additions & 0 deletions lightningd/dual_open_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,7 @@ wallet_update_channel(struct lightningd *ld,
inflight = new_inflight(channel,
NULL,
&channel->funding,
0,
funding_feerate,
channel->funding_sats,
channel->our_funds,
Expand Down Expand Up @@ -1500,6 +1501,7 @@ wallet_commit_channel(struct lightningd *ld,
inflight = new_inflight(channel,
NULL,
&channel->funding,
0,
funding_feerate,
channel->funding_sats,
channel->our_funds,
Expand Down
Loading
Loading