diff --git a/channeld/channeld.c b/channeld/channeld.c index fe7f7707f264..e889468272ab 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -431,8 +431,6 @@ static void check_mutual_splice_locked(struct peer *peer) fmt_bitcoin_txid(tmpctx, &peer->splice_state->locked_txid)); - peer->splice_state->await_commitment_succcess = true; - /* This splice_locked event is used, so reset the flags to false */ peer->splice_state->locked_ready[LOCAL] = false; peer->splice_state->locked_ready[REMOTE] = false; @@ -1670,8 +1668,6 @@ static void send_revocation(struct peer *peer, master_wait_sync_reply(tmpctx, peer, take(msg_for_master), WIRE_CHANNELD_GOT_COMMITSIG_REPLY); - peer->splice_state->await_commitment_succcess = false; - /* Now that the master has persisted the new commitment advance the HSMD * and fetch the revocation secret for the old one. */ msg = make_revocation_msg(peer, peer->next_index[LOCAL]-2, @@ -1990,30 +1986,44 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, peer_failed_warn(peer->pps, &peer->channel_id, "Bad commit_sig %s", tal_hex(msg, msg)); - /* BOLT-0d8b701614b09c6ee4172b04da2203e73deec7e2 #2: - * Once a node has received and sent `splice_locked`: - * - Until sending OR receiving of `revoke_and_ack` - * ... - * - MUST ignore `commitment_signed` messages where `splice_channel_id` - * does not match the `channel_id` of the confirmed splice. */ - if (peer->splice_state->await_commitment_succcess - && !tal_count(peer->splice_state->inflights) && cs_tlv && cs_tlv->splice_info) { - if (!bitcoin_txid_eq(&peer->channel->funding.txid, - cs_tlv->splice_info)) { - status_info("Ignoring stale commit_sig for channel_id" - " %s, as %s is locked in now.", - fmt_bitcoin_txid(tmpctx, - cs_tlv->splice_info), - fmt_bitcoin_txid(tmpctx, - &peer->channel->funding.txid)); - return NULL; - } + /* BOLT-f9fd539db6cc6f3e532fdc8cc1ebe8eb1a8fd717 + * - If the sending node sent `start_batch` and we are processing a batch of + * `commitment_signed` messages: + */ + if (msg_batch) { + + /* BOLT-f9fd539db6cc6f3e532fdc8cc1ebe8eb1a8fd717 + * - If `funding_txid` is missing in one of the `commitment_signed` messages: + * - MUST send an `error` and fail the channel. + */ + if (!cs_tlv->splice_info) + peer_failed_err(peer->pps, &peer->channel_id, + "Must send funding_txid when sending" + " a commitment batch."); + + /* BOLT-f9fd539db6cc6f3e532fdc8cc1ebe8eb1a8fd717 + * - Otherwise (no pending splice transactions): + *... + * - If `commitment_signed` is missing for the current funding transaction: + * - MUST send an `error` and fail the channel. + */ + if (!tal_count(peer->splice_state->inflights) + && !bitcoin_txid_eq(cs_tlv->splice_info, + &peer->channel->funding.txid)) + peer_failed_err(peer->pps, &peer->channel_id, + "Commitment batch is is missing our" + " current funding transaction %s", + fmt_bitcoin_txid(tmpctx, &peer->channel->funding.txid)); } - /* In a race we can get here with a commitsig with too many splices - * attached. In that case we ignore the main commit msg for the old - * funding tx, and for the splice candidates that didnt win. But we must - * listen to the one that is for the winning splice candidate */ + /* BOLT-f9fd539db6cc6f3e532fdc8cc1ebe8eb1a8fd717 + * - If `funding_txid` is missing in one of the `commitment_signed` messages: + * - MUST send an `error` and fail the channel. + */ + if (commit_index && !cs_tlv->splice_info) + peer_failed_err(peer->pps, &peer->channel_id, + "Must send funding_txid when sending" + " a commitment batch"); if (!changed_htlcs) { changed_htlcs = tal_arr(msg, const struct htlc *, 0); @@ -2108,7 +2118,6 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, "Bad commit_sig signature %"PRIu64" %s for tx" " %s wscript %s key %s feerate %u. Outpoint" " %s, funding_sats: %s, splice_info: %s," - " race_await_commit: %s," " inflight splice count: %zu", local_index, fmt_bitcoin_signature(msg, &commit_sig), @@ -2122,8 +2131,6 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, ? fmt_bitcoin_txid(tmpctx, cs_tlv->splice_info) : "N/A", - peer->splice_state->await_commitment_succcess ? "yes" - : "no", tal_count(peer->splice_state->inflights)); } @@ -2224,9 +2231,14 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, tal_count(peer->splice_state->inflights)); commitsigs = tal_arr(NULL, const struct commitsig*, 0); - /* We expect multiple consequtive commit_sig messages if we have - * inflight splices. Since consequtive is requred, we recurse for - * each expected message, blocking until all are received. */ + /* BOLT-f9fd539db6cc6f3e532fdc8cc1ebe8eb1a8fd717 + * - If there are pending splice transactions: + * - MUST validate each `commitment_signed` based on `funding_txid`. + * - If `commitment_signed` is missing for a funding transaction: + * - MUST send an `error` and fail the channel. + * - Otherwise: + * - MUST respond with a `revoke_and_ack` message. + */ for (i = 0; i < tal_count(peer->splice_state->inflights); i++) { s64 funding_diff = sats_diff(peer->splice_state->inflights[i]->amnt, peer->channel->funding_sats); @@ -2302,35 +2314,21 @@ static int commit_cmp(const void *a, const void *n, void *peer) int commit_index_a = commit_index_from_msg(*(u8**)a, peer); int commit_index_n = commit_index_from_msg(*(u8**)n, peer); - status_debug("commit_cmp a: %p, n: %p result: %d & %d", - *(u8**)a, *(u8**)n, commit_index_a, commit_index_n); - - if (commit_index_a == commit_index_n) { - status_debug("commit_cmp: return 0"); + if (commit_index_a == commit_index_n) return 0; - } - /* Unrecognized commits go on the end */ - if (commit_index_a == -1) { - status_debug("commit_cmp: return 1"); + if (commit_index_a == -1) return 1; - } - if (commit_index_n == -1) { - status_debug("commit_cmp: return -1"); + if (commit_index_n == -1) return -1; - } - status_debug("commit_cmp: return %d - %d = %d", - commit_index_a, commit_index_n, - commit_index_a - commit_index_n); /* Otherwise we sort by commit_index */ return commit_index_a - commit_index_n; } static struct commitsig_info *handle_peer_commit_sig_batch(struct peer *peer, const u8 *msg, - u32 commit_index, struct pubkey remote_funding, const struct htlc **changed_htlcs, s64 splice_amnt, @@ -2360,9 +2358,18 @@ static struct commitsig_info *handle_peer_commit_sig_batch(struct peer *peer, peer_failed_warn(peer->pps, &peer->channel_id, "Bad commit_sig %s", tal_hex(msg, msg)); + /* BOLT-f9fd539db6cc6f3e532fdc8cc1ebe8eb1a8fd717 + * - If there are pending splice transactions and the sending node did not + * send `start_batch` followed by a batch of `commitment_signed` messages: + * - MUST send an `error` and fail the channel. + */ + if (batch_size < 2 && last_inflight(peer)) + peer_failed_err(peer->pps, &peer->channel_id, "Must send a" + " commitment batch (ie. start_batch) when I" + " have pending splices inflight."); + msg_batch = tal_arr(tmpctx, const u8*, batch_size); msg_batch[0] = msg; - status_debug("msg_batch[0]: %p", msg_batch[0]); /* Already received commitment signed once, so start at i = 1 */ for (u16 i = 1; i < batch_size; i++) { @@ -2378,7 +2385,6 @@ static struct commitsig_info *handle_peer_commit_sig_batch(struct peer *peer, "Expected splice related " "WIRE_COMMITMENT_SIGNED but got %s", peer_wire_name(type)); - status_debug("fromwire_commitment_signed(%p) splice index %d", sub_msg, (int)i); if (!fromwire_commitment_signed(tmpctx, sub_msg, &channel_id, &commit_sig.s, &raw_sigs, &sub_cs_tlv)) @@ -2396,13 +2402,18 @@ static struct commitsig_info *handle_peer_commit_sig_batch(struct peer *peer, tal_hex(sub_msg, sub_msg), i, batch_size); msg_batch[i] = sub_msg; - status_debug("msg_batch[%d]: %p", (int)i, msg_batch[i]); } - status_debug("Sorting the msg_batch of tal_count %d, batch_size: %d", (int)tal_count(msg_batch), (int)batch_size); + /* BOLT-f9fd539db6cc6f3e532fdc8cc1ebe8eb1a8fd717 + * - Otherwise (no pending splice transactions): + * - MUST ignore `commitment_signed` where `funding_txid` does not match + * the current funding transaction. + */ + /* Sort puts all unrecognized `commitment_signed` messages onto the back + * of `msg_batch`, where they will be ignored */ asort(msg_batch, tal_count(msg_batch), commit_cmp, peer); - return handle_peer_commit_sig(peer, msg_batch[0], commit_index, + return handle_peer_commit_sig(peer, msg_batch[0], 0, remote_funding, changed_htlcs, splice_amnt, remote_splice_amnt, local_index, local_per_commit, @@ -2428,7 +2439,7 @@ static void handle_peer_start_batch(struct peer *peer, const u8 *msg) return; } - handle_peer_commit_sig_batch(peer, peer_read(tmpctx, peer->pps), 0, + handle_peer_commit_sig_batch(peer, peer_read(tmpctx, peer->pps), peer->channel->funding_pubkey[REMOTE], NULL, 0, 0, peer->next_index[LOCAL], @@ -2577,8 +2588,6 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg) fmt_pubkey(tmpctx, &peer->remote_per_commit), fmt_pubkey(tmpctx, &peer->old_remote_per_commit)); - peer->splice_state->await_commitment_succcess = false; - /* STFU can't be activated during pending updates. * With updates finish let's handle a potentially queued stfu request. */ @@ -4095,8 +4104,6 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg) peer->splicing->remote_funding_pubkey = last_inflight(peer)->remote_funding; } - peer->splice_state->await_commitment_succcess = false; - if (!is_stfu_active(peer)) peer_failed_warn(peer->pps, &peer->channel_id, "Must be in STFU mode before intiating splice"); @@ -4782,7 +4789,6 @@ static void handle_splice_stfu_success(struct peer *peer) init_rbf_tlvs); } - peer->splice_state->await_commitment_succcess = false; peer_write(peer->pps, take(msg)); } @@ -5000,7 +5006,7 @@ static void peer_in(struct peer *peer, const u8 *msg) handle_peer_start_batch(peer, msg); return; case WIRE_COMMITMENT_SIGNED: - handle_peer_commit_sig_batch(peer, msg, 0, + handle_peer_commit_sig_batch(peer, msg, peer->channel->funding_pubkey[REMOTE], NULL, 0, 0, peer->next_index[LOCAL], diff --git a/channeld/splice.c b/channeld/splice.c index e72b4af54830..a998ce4ae0d9 100644 --- a/channeld/splice.c +++ b/channeld/splice.c @@ -9,7 +9,6 @@ struct splice_state *splice_state_new(const tal_t *ctx) splice_state->count = 0; splice_state->locked_ready[LOCAL] = false; splice_state->locked_ready[REMOTE] = false; - splice_state->await_commitment_succcess = false; splice_state->inflights = NULL; splice_state->remote_locked_txid = NULL; diff --git a/channeld/splice.h b/channeld/splice.h index be6f7ff583d1..47050aee041b 100644 --- a/channeld/splice.h +++ b/channeld/splice.h @@ -17,8 +17,6 @@ struct splice_state { struct short_channel_id last_short_channel_id; /* Tally of which sides are locked, or not */ bool locked_ready[NUM_SIDES]; - /* Set to true when commitment cycle completes successfully */ - bool await_commitment_succcess; /* The txid of which splice inflight was confirmed */ struct bitcoin_txid locked_txid; /* The txid our peer locked their splice on */