diff --git a/channeld/channeld.c b/channeld/channeld.c index 11d8fdeea572..f391eddbd0b6 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -5951,19 +5951,30 @@ static void peer_reconnect(struct peer *peer, "next_funding_txid not recognized."); } + /* "none of those channel_reestablish messages contain + * my_current_funding_locked or next_funding for a splice transaction" */ + bool is_splice_active = local_next_funding + || peer->splice_state->locked_ready[LOCAL] + || 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)); + /* BOLT #2: * * - if `next_commitment_number` is 1 in both the - * `channel_reestablish` it sent and received: + * `channel_reestablish` it sent and received: * - MUST retransmit `channel_ready`. * - otherwise: - * - MUST NOT retransmit `channel_ready`, but MAY send - * `channel_ready` with a different `short_channel_id` - * `alias` field. + * - MUST NOT retransmit `channel_ready`, but MAY send `channel_ready` with + * a different `short_channel_id` `alias` field. */ if (peer->channel_ready[LOCAL] && peer->next_index[LOCAL] == 1 - && next_commitment_number == 1) { + && next_commitment_number == 1 + && !is_splice_active) { struct tlv_channel_ready_tlvs *tlvs = tlv_channel_ready_tlvs_new(tmpctx); tlvs->short_channel_id = &peer->local_alias; diff --git a/common/bech32_util.c b/common/bech32_util.c index 3722a19cb200..fd952b6ab468 100644 --- a/common/bech32_util.c +++ b/common/bech32_util.c @@ -79,7 +79,7 @@ bool from_bech32_charset(const tal_t *ctx, u5 *u5data; const char *sep; bool upper = false, lower = false; - size_t datalen; + size_t datalen, nbits, trailing; sep = memchr(bech32, '1', bech32_len); if (!sep) @@ -105,6 +105,18 @@ bool from_bech32_charset(const tal_t *ctx, if (upper && lower) goto fail; + /* Padding: converting N 5-bit groups to bytes leaves (N*5 % 8) trailing + * bits. These must be zero and fewer than 5 (otherwise a full 5-bit + * group is wasted as padding, which is invalid). */ + nbits = datalen * 5; + trailing = nbits % 8; + if (trailing >= 5) + goto fail; + for (size_t i = nbits - trailing; i < nbits; i++) { + if (get_u5_bit(u5data, i)) + goto fail; + } + *data = tal_arr(ctx, u8, 0); if (!bech32_pull_bits(data, u5data, tal_bytelen(u5data) * 5)) { tal_free(*data); diff --git a/common/bolt11.c b/common/bolt11.c index 26263dd028c0..7a1f12db55bc 100644 --- a/common/bolt11.c +++ b/common/bolt11.c @@ -1045,7 +1045,7 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str, &sig, (const u8 *)&hash)) return decode_fail(b11, fail, - "signature recovery failed"); + "public-key recovery failed"); node_id_from_pubkey(&b11->receiver_id, &k); } else { struct pubkey k; diff --git a/common/bolt12.c b/common/bolt12.c index 1f2ccf1b659d..6043bd676612 100644 --- a/common/bolt12.c +++ b/common/bolt12.c @@ -10,7 +10,8 @@ #include #include -/* If chains is NULL, max_num_chains is ignored */ +/* If chains is NULL, max_num_chains is ignored. + * If must_be_chain is NULL, only structural validity is checked. */ bool bolt12_chains_match(const struct bitcoin_blkid *chains, size_t max_num_chains, const struct chainparams *must_be_chain) @@ -31,6 +32,13 @@ bool bolt12_chains_match(const struct bitcoin_blkid *chains, * - if the node does not accept invoices for at least one of the `chains`: * - MUST NOT respond to the offer */ + if (chains && max_num_chains == 0) + return false; + + /* No specific chain required: structurally valid. */ + if (!must_be_chain) + return true; + if (!chains) { max_num_chains = 1; chains = &chainparams_for_network("bitcoin")->genesis_blockhash; @@ -187,6 +195,18 @@ struct tlv_offer *offer_decode(const tal_t *ctx, return NULL; } + /* BOLT #12: + * - otherwise: (`offer_chains` is set): + * - if the node does not accept invoices for at least one of the `chains`: + * - MUST NOT respond to the offer + */ + for (size_t i = 0; i < tal_count(offer->fields); i++) { + if (offer->fields[i].numtype == 2 && offer->fields[i].length == 0) { + *fail = tal_strdup(ctx, "offer_chains must have at least one entry"); + return tal_free(offer); + } + } + *fail = check_features_and_chain(ctx, our_features, must_be_chain, offer->offer_features, diff --git a/common/features.h b/common/features.h index 2861573647ea..6255340103fe 100644 --- a/common/features.h +++ b/common/features.h @@ -113,7 +113,7 @@ struct feature_set *feature_set_dup(const tal_t *ctx, * | 16/17 | `basic_mpp` |... IN9 ... * | 18/19 | `option_support_large_channel` |... IN ... * | 22/23 | `option_anchors` |... INT ... - * | 24/25 | `option_route_blinding` |...IN9 ... + * | 24/25 | `option_route_blinding` |... IN9 ... * | 26/27 | `option_shutdown_anysegwit` |... IN ... * | 28/29 | `option_dual_fund` |... IN ... * | 34/35 | `option_quiesce` |... IN ... diff --git a/common/test/run-bolt11.c b/common/test/run-bolt11.c index 33b228bc7df8..3f5fdcb81890 100644 --- a/common/test/run-bolt11.c +++ b/common/test/run-bolt11.c @@ -873,7 +873,7 @@ int main(int argc, char *argv[]) * > lnbc2500u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpusp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9qrsgqwgt7mcn5yqw3yx0w94pswkpq6j9uh6xfqqqtsk4tnarugeektd4hg5975x9am52rz4qskukxdmjemg92vvqz8nvmsye63r5ykel43pgz7zq0g2 */ assert(!bolt11_decode(tmpctx, "lnbc2500u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpusp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9qrsgqwgt7mcn5yqw3yx0w94pswkpq6j9uh6xfqqqtsk4tnarugeektd4hg5975x9am52rz4qskukxdmjemg92vvqz8nvmsye63r5ykel43pgz7zq0g2", NULL, NULL, NULL, &fail)); - assert(streq(fail, "signature recovery failed")); + assert(streq(fail, "public-key recovery failed")); /* BOLT #11: * > ### String is too short. diff --git a/common/test/run-bolt12-encode-test.c b/common/test/run-bolt12-encode-test.c index 9391b1a38a08..dadc715b30c8 100644 --- a/common/test/run-bolt12-encode-test.c +++ b/common/test/run-bolt12-encode-test.c @@ -429,15 +429,18 @@ int main(int argc, char *argv[]) /* BOLT #12: * - if `offer_amount` is set and `offer_description` is not set: * - MUST NOT respond to the offer. - * - if `offer_amount` is set and is not greater than zero: + * - if `offer_amount` is set and is not greater than zero: * - MUST NOT respond to the offer. * - if `offer_currency` is set and `offer_amount` is not set: * - MUST NOT respond to the offer. * - if neither `offer_issuer_id` nor `offer_paths` are set: * - MUST NOT respond to the offer. */ + offer->offer_amount = tal(offer, u64); + *offer->offer_amount = 10000; + offer->offer_description = NULL; - print_invalid_offer(offer, "Missing offer_description and offer_amount"); + print_invalid_offer(offer, "Missing offer_description"); offer->offer_description = tal_utf8(tmpctx, "Test vectors"); offer->offer_amount = tal(offer, u64); diff --git a/common/test/run-bolt12-format-string-test.c b/common/test/run-bolt12-format-string-test.c index 10f6748b2ddd..d861b3aaaa01 100644 --- a/common/test/run-bolt12-format-string-test.c +++ b/common/test/run-bolt12-format-string-test.c @@ -128,7 +128,7 @@ int main(int argc, char *argv[]) * - SHOULD omit `offer_chains`, implying that bitcoin is only chain. * - if a specific minimum `offer_amount` is required for successful payment: * - MUST set `offer_amount` to the amount expected (per item). - * - MUST set `offer_amount` greater than zero. + * - MUST set `offer_amount` greater than zero. * - if the currency for `offer_amount` is that of all entries in `chains`: * - MUST specify `offer_amount` in multiples of the minimum lightning-payable unit * (e.g. milli-satoshis for bitcoin). diff --git a/lightningd/channel_gossip.c b/lightningd/channel_gossip.c index 63088b17c209..53c67e7109f5 100644 --- a/lightningd/channel_gossip.c +++ b/lightningd/channel_gossip.c @@ -687,10 +687,10 @@ static void stash_remote_announce_sigs(struct channel *channel, * A node: * - If the `open_channel` message has the `announce_channel` bit set AND a * `shutdown` message has not been sent: - * - After `channel_ready` has been sent and received AND the funding - * transaction has enough confirmations to ensure that it won't be - * reorganized: - * - MUST send `announcement_signatures` for the funding transaction. + * - After `channel_ready` has been sent and received AND the funding + * transaction has enough confirmations to ensure that it won't be + * reorganized: + * - MUST send `announcement_signatures` for the funding transaction.... * - Otherwise: * - MUST NOT send the `announcement_signatures` message. */ diff --git a/openingd/openingd.c b/openingd/openingd.c index 52e50572e2e0..ac9752a2b694 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -332,12 +332,12 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags, = state->upfront_shutdown_script[LOCAL]; /* BOLT #2: - * - MUST set `channel_type`: - * - MUST set it to a defined type representing the type it wants. - * - MUST use the smallest bitmap possible to represent the channel - * type. - * - SHOULD NOT set it to a type containing a feature which was not - * negotiated. + * - MUST set `channel_type`: + * - MUST set it to a defined type representing the type it wants. + * - MUST use the smallest bitmap possible to represent the channel + * type. + * - SHOULD NOT set it to a type containing a feature which was not + * negotiated. */ open_tlvs->channel_type = state->channel_type->features; diff --git a/tests/test_splicing.py b/tests/test_splicing.py index f713ff7ba14e..6a644b6d618d 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -416,6 +416,8 @@ def test_commit_crash_splice(node_factory, bitcoind): l1.daemon.wait_for_log(r"Splice initiator: we commit") + # Snapshot log position before restart so we only search post-restart logs below. + pre_restart_logpos = l1.daemon.logsearch_start l1.restart() # The splicing inflight should have been left pending in the DB @@ -426,6 +428,11 @@ def test_commit_crash_splice(node_factory, bitcoind): l1.daemon.wait_for_log(r'Splice resume check with local_next_funding: sent, remote_next_funding: received, inflights: 1') l1.daemon.wait_for_log(r'Splice negotation, will not send commit, not recv commit, send signature, recv signature as initiator') + # channel_ready MUST NOT be retransmitted when either channel_reestablish + # message contains next_funding or my_current_funding_locked + assert not l1.daemon.is_in_log(r'Retransmitting channel_ready for channel', + start=pre_restart_logpos) + assert l1.db_query("SELECT count(*) as c FROM channel_funding_inflights;")[0]['c'] == 1 l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 0eb0433c6ff3..86377087b08d 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -2548,11 +2548,11 @@ def test_unspend_during_reorg(node_factory, bitcoind): blockheight, txindex, _ = scid.split('x') # Use mainnet settings for rescan. - l3 = node_factory.get_node(options={'rescan': 15}) + l3 = node_factory.get_node(options={'rescan': 75}) l3.connect(l2) mine_funding_to_announce(bitcoind, [l1, l2, l3]) - bitcoind.generate_block(20) + bitcoind.generate_block(90) sync_blockheight(bitcoind, [l3]) wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 2) @@ -2565,7 +2565,7 @@ def test_unspend_during_reorg(node_factory, bitcoind): bitcoind.generate_block(74, wait_for_mempool=1) wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 2) - # In one fell swoop it goes through dying, to dead (12 blocks) + # In one fell swoop it goes through dying, to dead (72 blocks) l3.daemon.wait_for_log(f"Adding block {spentheight}") l3.daemon.wait_for_log(f"gossipd: channel {scid} closing soon due to the funding outpoint being spent") l3.daemon.wait_for_log(f"gossipd: Deleting channel {scid} due to the funding outpoint being spent")