Bolt quote updates and improvements

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;

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);

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;

common/bolt12.c

@@ -10,7 +10,8 @@
 #include <inttypes.h>
 #include <time.h>
 
-/* 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,

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 ...

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.

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);

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).

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.
  */

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;
 

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')

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")