Revert "Bolt quote updates and improvements"
What changed, and why it matters
This commit is a wholesale revert of an earlier pull request that was accidentally merged without proper review. It removes new splice-handling logic in the channel daemon, stricter bech32 padding checks, and several BOLT 11/12 quote and validation updates. The revert itself is a defensive cleanup: it restores the older, reviewed code because the maintainers decided the new approach was not spec-compliant. It is not a security patch in the usual sense, but it does undo code that had been rejected as incorrect, reducing the risk of protocol misbehavior.
Treat this as a code-quality/revert commit rather than an urgent security fix. Review the replacement pull request once it is opened to ensure the spec-compliant funding_tx_index approach is implemented and reviewed before merge. Monitor for any follow-up security advisories from the Core Lightning team, but none are indicated here.
Security signals we found
Reverts unreviewed code that was rejected as spec-deviating
Removes txid-comparison splice detection heuristic in channel reconnection
Removes stricter bech32 padding-bit validation
No explicit security bug or vulnerability described
Evidence from the diff
Commit 0a8a7afec0f373177912207fbf96b67f77ca5fc7 reverts commits b0e728572b through aa6ecad90e (11 commits). The most substantive functional change is in channeld/channeld.c: the newly introduced is_splice_active heuristic (which compared txids to decide whether a splice was active during reconnection) is removed, restoring the simpler channel_ready retransmission logic. The commit message explicitly states this logic was NACKed by @ddustin because it deviates from the BOLT spec; the correct fix requires a funding_tx_index on the inflight rather than a txid comparison. Other reverted changes include stricter bech32 padding validation in common/bech32_util.c, BOLT quote formatting and comment updates, and test adjustments. No CVE, advisory, or independent researcher attribution is present in the commit or supplied references.
Changed components
channeld/channeld.ccommon/bech32_util.ccommon/bolt11.ccommon/bolt12.ccommon/features.hcommon/test/run-bolt11.ccommon/test/run-bolt12-encode-test.ccommon/test/run-bolt12-format-string-test.clightningd/channel_gossip.copeningd/openingd.ctests/test_splicing.pytests/test_wallet.pyInspect captured patch +26 / −79
diff --git a/channeld/channeld.c b/channeld/channeld.c
index 742ac704..ed4cc6e8 100644
--- a/channeld/channeld.c
+++ b/channeld/channeld.c
@@ -5954,30 +5954,19 @@ 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
- && !is_splice_active) {
+ && next_commitment_number == 1) {
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 fd952b6a..3722a19c 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, nbits, trailing;
+ size_t datalen;
sep = memchr(bech32, '1', bech32_len);
if (!sep)
@@ -105,18 +105,6 @@ 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 e4d8630d..bc851ed4 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,
- "public-key recovery failed");
+ "signature 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 4b75af62..3b80f057 100644
--- a/common/bolt12.c
+++ b/common/bolt12.c
@@ -10,8 +10,7 @@
#include <inttypes.h>
#include <time.h>
-/* If chains is NULL, max_num_chains is ignored.
- * If must_be_chain is NULL, only structural validity is checked. */
+/* If chains is NULL, max_num_chains is ignored */
bool bolt12_chains_match(const struct bitcoin_blkid *chains,
size_t max_num_chains,
const struct chainparams *must_be_chain)
@@ -32,13 +31,6 @@ 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;
@@ -197,18 +189,6 @@ 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 62553401..28615736 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 7caf2edc..a3c4c3a2 100644
--- a/common/test/run-bolt11.c
+++ b/common/test/run-bolt11.c
@@ -874,7 +874,7 @@ int main(int argc, char *argv[])
* > lnbc2500u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpusp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9qrsgqwgt7mcn5yqw3yx0w94pswkpq6j9uh6xfqqqtsk4tnarugeektd4hg5975x9am52rz4qskukxdmjemg92vvqz8nvmsye63r5ykel43pgz7zq0g2
*/
assert(!bolt11_decode(tmpctx, "lnbc2500u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpusp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9qrsgqwgt7mcn5yqw3yx0w94pswkpq6j9uh6xfqqqtsk4tnarugeektd4hg5975x9am52rz4qskukxdmjemg92vvqz8nvmsye63r5ykel43pgz7zq0g2", NULL, NULL, NULL, &fail));
- assert(streq(fail, "public-key recovery failed"));
+ assert(streq(fail, "signature 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 4692c5b8..aff41967 100644
--- a/common/test/run-bolt12-encode-test.c
+++ b/common/test/run-bolt12-encode-test.c
@@ -429,18 +429,15 @@ 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");
+ print_invalid_offer(offer, "Missing offer_description and offer_amount");
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 d861b3aa..10f6748b 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 53c67e71..63088b17 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 ac9752a2..52e50572 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 8cf4e724..8e9ba0e4 100644
--- a/tests/test_splicing.py
+++ b/tests/test_splicing.py
@@ -416,8 +416,6 @@ 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
@@ -428,11 +426,6 @@ 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 893e55e1..651eb0e5 100644
--- a/tests/test_wallet.py
+++ b/tests/test_wallet.py
@@ -2648,11 +2648,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': 75})
+ l3 = node_factory.get_node(options={'rescan': 15})
l3.connect(l2)
mine_funding_to_announce(bitcoind, [l1, l2, l3])
- bitcoind.generate_block(90)
+ bitcoind.generate_block(20)
sync_blockheight(bitcoind, [l3])
wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 2)
@@ -2665,7 +2665,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 (72 blocks)
+ # In one fell swoop it goes through dying, to dead (12 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")
Why this scored 26/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.