Add an upgrade test of splicing after upgrading from 0.1
What changed, and why it matters
This commit adds a regression test for a splicing bug in the Lightning Dev Kit. The bug could leave a forwarded payment (HTLC) in a dangling state if a channel was spliced while the payment was still queued for forwarding. The test verifies that after upgrading from LDK 0.1, such HTLCs are correctly failed or claimed. The commit also exposes some internal test helpers so the new cross-version test can use them. It does not itself change production logic, so it is primarily a test/verification patch rather than a fix.
Treat this commit as test-hardening rather than an active vulnerability fix. Review whether the underlying production-code fixes for the two described splice defects are already present in the branch; if not, ensure they are tracked and landed separately. Consider running the new upgrade/downgrade test in CI to prevent regressions.
Security signals we found
Regression test for dangling HTLC after splice
Historical bug: backwards HTLC failure used stale SCID after splice
Historical bug: KeysManager attempted to_remote key rotation during splice
Cross-version upgrade/downgrade test coverage added
Internal test-only visibility changes (no production code path changes)
Evidence from the diff
The commit introduces do_test_0_1_htlc_forward_after_splice in lightning-tests/src/upgrade_downgrade_tests.rs. It simulates a three-node route (A→B→C) created in LDK 0.1, serializes the nodes, reloads them under the current LDK, splices the A→B channel, and then exercises both the fail-back and success paths for the HTLC that was pending in B’s forwarding queue. The commit message notes two historical splice defects: (1) a failed backwards HTLC using the original short channel ID (SCID) which becomes invalid after splice, potentially causing a dangling HTLC; and (2) a KeysManager issue where rotating the to_remote key was attempted during splicing, which the protocol does not support. To enable the new test, several helpers in lightning/src/ln/splicing_tests.rs are made pub and the module is exposed under _test_utils, and HTLC_FAIL_BACK_BUFFER is made pub in channelmonitor.rs. The Cargo.toml dependency on the old LDK version is bumped from 0.1.1 to 0.1.7.
Changed components
lightning-tests/src/upgrade_downgrade_tests.rslightning-tests/Cargo.tomllightning/src/ln/splicing_tests.rslightning/src/ln/mod.rslightning/src/chain/channelmonitor.rsInspect captured patch +221 / −15
diff --git a/lightning-tests/Cargo.toml b/lightning-tests/Cargo.toml
index 23c81fa..36b03a2 100644
--- a/lightning-tests/Cargo.toml
+++ b/lightning-tests/Cargo.toml
@@ -14,7 +14,7 @@ lightning-types = { path = "../lightning-types", features = ["_test_utils"] }
lightning-invoice = { path = "../lightning-invoice", default-features = false }
lightning-macros = { path = "../lightning-macros" }
lightning = { path = "../lightning", features = ["_test_utils"] }
-lightning_0_1 = { package = "lightning", version = "0.1.1", features = ["_test_utils"] }
+lightning_0_1 = { package = "lightning", version = "0.1.7", features = ["_test_utils"] }
lightning_0_0_125 = { package = "lightning", version = "0.0.125", features = ["_test_utils"] }
bitcoin = { version = "0.32.2", default-features = false }
diff --git a/lightning-tests/src/upgrade_downgrade_tests.rs b/lightning-tests/src/upgrade_downgrade_tests.rs
index 0b3f5df..d1153b7 100644
--- a/lightning-tests/src/upgrade_downgrade_tests.rs
+++ b/lightning-tests/src/upgrade_downgrade_tests.rs
@@ -10,9 +10,15 @@
//! Tests which test upgrading from previous versions of LDK or downgrading to previous versions of
//! LDK.
+use lightning_0_1::commitment_signed_dance as commitment_signed_dance_0_1;
use lightning_0_1::events::ClosureReason as ClosureReason_0_1;
+use lightning_0_1::expect_pending_htlcs_forwardable_ignore as expect_pending_htlcs_forwardable_ignore_0_1;
use lightning_0_1::get_monitor as get_monitor_0_1;
+use lightning_0_1::ln::channelmanager::PaymentId as PaymentId_0_1;
+use lightning_0_1::ln::channelmanager::RecipientOnionFields as RecipientOnionFields_0_1;
use lightning_0_1::ln::functional_test_utils as lightning_0_1_utils;
+use lightning_0_1::ln::msgs::ChannelMessageHandler as _;
+use lightning_0_1::routing::router as router_0_1;
use lightning_0_1::util::ser::Writeable as _;
use lightning_0_0_125::chain::ChannelMonitorUpdateStatus as ChannelMonitorUpdateStatus_0_0_125;
@@ -29,16 +35,23 @@ use lightning_0_0_125::ln::msgs::ChannelMessageHandler as _;
use lightning_0_0_125::routing::router as router_0_0_125;
use lightning_0_0_125::util::ser::Writeable as _;
-use lightning::chain::channelmonitor::ANTI_REORG_DELAY;
-use lightning::events::{ClosureReason, Event};
+use lightning::chain::channelmonitor::{ANTI_REORG_DELAY, HTLC_FAIL_BACK_BUFFER};
+use lightning::events::bump_transaction::sync::WalletSourceSync;
+use lightning::events::{ClosureReason, Event, HTLCHandlingFailureType};
use lightning::ln::functional_test_utils::*;
+use lightning::ln::funding::SpliceContribution;
+use lightning::ln::msgs::BaseMessageHandler as _;
+use lightning::ln::msgs::ChannelMessageHandler as _;
+use lightning::ln::msgs::MessageSendEvent;
+use lightning::ln::splicing_tests::*;
+use lightning::ln::types::ChannelId;
use lightning::sign::OutputSpender;
-use lightning_types::payment::PaymentPreimage;
+use lightning_types::payment::{PaymentHash, PaymentPreimage, PaymentSecret};
-use bitcoin::opcodes;
use bitcoin::script::Builder;
use bitcoin::secp256k1::Secp256k1;
+use bitcoin::{opcodes, Amount, TxOut};
use std::sync::Arc;
@@ -299,3 +312,188 @@ fn test_0_1_legacy_remote_key_derivation() {
panic!("Wrong event");
}
}
+
+fn do_test_0_1_htlc_forward_after_splice(fail_htlc: bool) {
+ // Test what happens if an HTLC set to be forwarded in 0.1 is forwarded after the inbound
+ // channel is spliced. In the initial splice code, this could have led to a dangling HTLC if
+ // the HTLC is failed as the backwards-failure would use the channel's original SCID which is
+ // no longer valid.
+ // In some later splice code, this also failed because the `KeysManager` would have tried to
+ // rotate the `to_remote` key, which we aren't able to do in the splicing protocol.
+ let (node_a_ser, node_b_ser, node_c_ser, mon_a_1_ser, mon_b_1_ser, mon_b_2_ser, mon_c_1_ser);
+ let (node_a_id, node_b_id, node_c_id);
+ let (chan_id_bytes_a, chan_id_bytes_b);
+ let (payment_secret_bytes, payment_hash_bytes, payment_preimage_bytes);
+ let (node_a_blocks, node_b_blocks, node_c_blocks);
+
+ const EXTRA_BLOCKS_BEFORE_FAIL: u32 = 145;
+
+ {
+ let chanmon_cfgs = lightning_0_1_utils::create_chanmon_cfgs(3);
+ let node_cfgs = lightning_0_1_utils::create_node_cfgs(3, &chanmon_cfgs);
+ let node_chanmgrs =
+ lightning_0_1_utils::create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+ let nodes = lightning_0_1_utils::create_network(3, &node_cfgs, &node_chanmgrs);
+
+ node_a_id = nodes[0].node.get_our_node_id();
+ node_b_id = nodes[1].node.get_our_node_id();
+ node_c_id = nodes[2].node.get_our_node_id();
+ let chan_id_a = lightning_0_1_utils::create_announced_chan_between_nodes_with_value(
+ &nodes, 0, 1, 10_000_000, 0,
+ )
+ .2;
+ chan_id_bytes_a = chan_id_a.0;
+
+ let chan_id_b = lightning_0_1_utils::create_announced_chan_between_nodes_with_value(
+ &nodes, 1, 2, 50_000, 0,
+ )
+ .2;
+ chan_id_bytes_b = chan_id_b.0;
+
+ // Ensure all nodes are at the same initial height.
+ let node_max_height = nodes.iter().map(|node| node.best_block_info().1).max().unwrap();
+ for node in &nodes {
+ let blocks_to_mine = node_max_height - node.best_block_info().1;
+ if blocks_to_mine > 0 {
+ lightning_0_1_utils::connect_blocks(node, blocks_to_mine);
+ }
+ }
+
+ let (preimage, hash, secret) =
+ lightning_0_1_utils::get_payment_preimage_hash(&nodes[2], Some(1_000_000), None);
+ payment_preimage_bytes = preimage.0;
+ payment_hash_bytes = hash.0;
+ payment_secret_bytes = secret.0;
+
+ let pay_params = router_0_1::PaymentParameters::from_node_id(
+ node_c_id,
+ lightning_0_1_utils::TEST_FINAL_CLTV,
+ )
+ .with_bolt11_features(nodes[2].node.bolt11_invoice_features())
+ .unwrap();
+
+ let route_params =
+ router_0_1::RouteParameters::from_payment_params_and_value(pay_params, 1_000_000);
+ let mut route = lightning_0_1_utils::get_route(&nodes[0], &route_params).unwrap();
+ route.paths[0].hops[1].cltv_expiry_delta =
+ EXTRA_BLOCKS_BEFORE_FAIL + HTLC_FAIL_BACK_BUFFER + 1;
+ if fail_htlc {
+ // Pay more than the channel's value (and probably not enough fee)
+ route.paths[0].hops[1].fee_msat = 50_000_000;
+ }
+
+ let onion = RecipientOnionFields_0_1::secret_only(secret);
+ let id = PaymentId_0_1(hash.0);
+ nodes[0].node.send_payment_with_route(route, hash, onion, id).unwrap();
+
+ lightning_0_1_utils::check_added_monitors(&nodes[0], 1);
+ let send_event = lightning_0_1_utils::SendEvent::from_node(&nodes[0]);
+
+ nodes[1].node.handle_update_add_htlc(node_a_id, &send_event.msgs[0]);
+ commitment_signed_dance_0_1!(nodes[1], nodes[0], send_event.commitment_msg, false);
+ expect_pending_htlcs_forwardable_ignore_0_1!(nodes[1]);
+
+ // We now have an HTLC pending in node B's forwarding queue with the original channel's
+ // SCID as the source.
+ // We now upgrade to 0.2 and splice before forwarding that HTLC...
+ node_a_ser = nodes[0].node.encode();
+ node_b_ser = nodes[1].node.encode();
+ node_c_ser = nodes[2].node.encode();
+ mon_a_1_ser = get_monitor_0_1!(nodes[0], chan_id_a).encode();
+ mon_b_1_ser = get_monitor_0_1!(nodes[1], chan_id_a).encode();
+ mon_b_2_ser = get_monitor_0_1!(nodes[1], chan_id_b).encode();
+ mon_c_1_ser = get_monitor_0_1!(nodes[2], chan_id_b).encode();
+
+ node_a_blocks = Arc::clone(&nodes[0].blocks);
+ node_b_blocks = Arc::clone(&nodes[1].blocks);
+ node_c_blocks = Arc::clone(&nodes[2].blocks);
+ }
+
+ // Create a dummy node to reload over with the 0.1 state
+ let mut chanmon_cfgs = create_chanmon_cfgs(3);
+
+ // Our TestChannelSigner will fail as we're jumping ahead, so disable its state-based checks
+ chanmon_cfgs[0].keys_manager.disable_all_state_policy_checks = true;
+ chanmon_cfgs[1].keys_manager.disable_all_state_policy_checks = true;
+ chanmon_cfgs[2].keys_manager.disable_all_state_policy_checks = true;
+
+ chanmon_cfgs[0].tx_broadcaster.blocks = node_a_blocks;
+ chanmon_cfgs[1].tx_broadcaster.blocks = node_b_blocks;
+ chanmon_cfgs[2].tx_broadcaster.blocks = node_c_blocks;
+
+ let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+ let (persister_a, persister_b, persister_c, chain_mon_a, chain_mon_b, chain_mon_c);
+ let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+ let (node_a, node_b, node_c);
+ let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+ let config = test_default_channel_config();
+ let a_mons = &[&mon_a_1_ser[..]];
+ reload_node!(nodes[0], config.clone(), &node_a_ser, a_mons, persister_a, chain_mon_a, node_a);
+ let b_mons = &[&mon_b_1_ser[..], &mon_b_2_ser[..]];
+ reload_node!(nodes[1], config.clone(), &node_b_ser, b_mons, persister_b, chain_mon_b, node_b);
+ let c_mons = &[&mon_c_1_ser[..]];
+ reload_node!(nodes[2], config, &node_c_ser, c_mons, persister_c, chain_mon_c, node_c);
+
+ reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
+ let mut reconnect_b_c_args = ReconnectArgs::new(&nodes[1], &nodes[2]);
+ reconnect_b_c_args.send_channel_ready = (true, true);
+ reconnect_b_c_args.send_announcement_sigs = (true, true);
+ reconnect_nodes(reconnect_b_c_args);
+
+ let contribution = SpliceContribution::SpliceOut {
+ outputs: vec![TxOut {
+ value: Amount::from_sat(1_000),
+ script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
+ }],
+ };
+ let splice_tx = splice_channel(&nodes[0], &nodes[1], ChannelId(chan_id_bytes_a), contribution);
+ for node in nodes.iter() {
+ mine_transaction(node, &splice_tx);
+ connect_blocks(node, ANTI_REORG_DELAY - 1);
+ }
+
+ let splice_locked = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceLocked, node_b_id);
+ lock_splice(&nodes[0], &nodes[1], &splice_locked, false);
+
+ for node in nodes.iter() {
+ connect_blocks(node, EXTRA_BLOCKS_BEFORE_FAIL - ANTI_REORG_DELAY);
+ }
+
+ // Now release the HTLC to be failed back to node A
+ nodes[1].node.process_pending_htlc_forwards();
+
+ let pay_secret = PaymentSecret(payment_secret_bytes);
+ let pay_hash = PaymentHash(payment_hash_bytes);
+ let pay_preimage = PaymentPreimage(payment_preimage_bytes);
+
+ if fail_htlc {
+ let failure = HTLCHandlingFailureType::Forward {
+ node_id: Some(node_c_id),
+ channel_id: ChannelId(chan_id_bytes_b),
+ };
+ expect_and_process_pending_htlcs_and_htlc_handling_failed(&nodes[1], &[failure]);
+ check_added_monitors(&nodes[1], 1);
+
+ let updates = get_htlc_update_msgs(&nodes[1], &node_a_id);
+ nodes[0].node.handle_update_fail_htlc(node_b_id, &updates.update_fail_htlcs[0]);
+ commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false);
+ let conditions = PaymentFailedConditions::new();
+ expect_payment_failed_conditions(&nodes[0], pay_hash, false, conditions);
+ } else {
+ check_added_monitors(&nodes[1], 1);
+ let forward_event = SendEvent::from_node(&nodes[1]);
+ nodes[2].node.handle_update_add_htlc(node_b_id, &forward_event.msgs[0]);
+ commitment_signed_dance!(nodes[2], nodes[1], forward_event.commitment_msg, false);
+
+ expect_and_process_pending_htlcs(&nodes[2], false);
+ expect_payment_claimable!(nodes[2], pay_hash, pay_secret, 1_000_000);
+ claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], pay_preimage);
+ }
+}
+
+#[test]
+fn test_0_1_htlc_forward_after_splice() {
+ do_test_0_1_htlc_forward_after_splice(true);
+ do_test_0_1_htlc_forward_after_splice(false);
+}
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index e2f0eff..4acba6f 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -327,7 +327,7 @@ pub const ARCHIVAL_DELAY_BLOCKS: u32 = 4032;
/// (2) is the same, but with an additional buffer to avoid accepting an HTLC which is immediately
/// in a race condition between the user connecting a block (which would fail it) and the user
/// providing us the preimage (which would claim it).
-pub(crate) const HTLC_FAIL_BACK_BUFFER: u32 = CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS;
+pub const HTLC_FAIL_BACK_BUFFER: u32 = CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS;
// Deprecated, use [`HolderCommitment`] or [`HolderCommitmentTransaction`].
#[derive(Clone, PartialEq, Eq)]
diff --git a/lightning/src/ln/mod.rs b/lightning/src/ln/mod.rs
index bae5558..9473142 100644
--- a/lightning/src/ln/mod.rs
+++ b/lightning/src/ln/mod.rs
@@ -120,8 +120,8 @@ mod reorg_tests;
#[cfg(test)]
#[allow(unused_mut)]
mod shutdown_tests;
-#[cfg(test)]
-mod splicing_tests;
+#[cfg(any(feature = "_test_utils", test))]
+pub mod splicing_tests;
#[cfg(any(test, feature = "_externalize_tests"))]
#[allow(unused_mut)]
pub mod update_fee_tests;
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index 2b415a1..f95883e 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -7,6 +7,8 @@
// You may not use this file except in accordance with one or both of these
// licenses.
+#![cfg_attr(not(test), allow(unused_imports))]
+
use crate::chain::chaininterface::FEERATE_FLOOR_SATS_PER_KW;
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS};
use crate::chain::transaction::OutPoint;
@@ -68,7 +70,7 @@ fn test_v1_splice_in_negative_insufficient_inputs() {
}
}
-fn negotiate_splice_tx<'a, 'b, 'c, 'd>(
+pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>(
initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId,
initiator_contribution: SpliceContribution,
) -> msgs::CommitmentSigned {
@@ -83,7 +85,7 @@ fn negotiate_splice_tx<'a, 'b, 'c, 'd>(
)
}
-fn complete_splice_handshake<'a, 'b, 'c, 'd>(
+pub fn complete_splice_handshake<'a, 'b, 'c, 'd>(
initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId,
initiator_contribution: SpliceContribution,
) -> ScriptBuf {
@@ -120,7 +122,7 @@ fn complete_splice_handshake<'a, 'b, 'c, 'd>(
new_funding_script
}
-fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>(
+pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>(
initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId,
initiator_contribution: SpliceContribution, new_funding_script: ScriptBuf,
) -> msgs::CommitmentSigned {
@@ -209,7 +211,7 @@ fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>(
}
}
-fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>(
+pub fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>(
initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>,
initial_commit_sig_for_acceptor: msgs::CommitmentSigned, is_0conf: bool,
) -> (Transaction, Option<(msgs::SpliceLocked, PublicKey)>) {
@@ -277,7 +279,7 @@ fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>(
(tx, splice_locked)
}
-fn splice_channel<'a, 'b, 'c, 'd>(
+pub fn splice_channel<'a, 'b, 'c, 'd>(
initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId,
initiator_contribution: SpliceContribution,
) -> Transaction {
@@ -304,7 +306,7 @@ fn splice_channel<'a, 'b, 'c, 'd>(
splice_tx
}
-fn lock_splice_after_blocks<'a, 'b, 'c, 'd>(
+pub fn lock_splice_after_blocks<'a, 'b, 'c, 'd>(
node_a: &'a Node<'b, 'c, 'd>, node_b: &'a Node<'b, 'c, 'd>, num_blocks: u32,
) {
connect_blocks(node_a, num_blocks);
@@ -316,7 +318,7 @@ fn lock_splice_after_blocks<'a, 'b, 'c, 'd>(
lock_splice(node_a, node_b, &splice_locked_for_node_b, false);
}
-fn lock_splice<'a, 'b, 'c, 'd>(
+pub fn lock_splice<'a, 'b, 'c, 'd>(
node_a: &'a Node<'b, 'c, 'd>, node_b: &'a Node<'b, 'c, 'd>,
splice_locked_for_node_b: &msgs::SpliceLocked, is_0conf: bool,
) {
@@ -387,6 +389,7 @@ fn test_splice_state_reset_on_disconnect() {
do_test_splice_state_reset_on_disconnect(true);
}
+#[cfg(test)]
fn do_test_splice_state_reset_on_disconnect(reload: bool) {
// Tests that we're able to forget our pending splice state after a disconnect such that we can
// retry later.
@@ -714,6 +717,7 @@ fn test_splice_out() {
let _ = send_payment(&nodes[0], &[&nodes[1]], htlc_limit_msat);
}
+#[cfg(test)]
#[derive(PartialEq)]
enum SpliceStatus {
Unconfirmed,
@@ -731,6 +735,7 @@ fn test_splice_commitment_broadcast() {
do_test_splice_commitment_broadcast(SpliceStatus::Locked, true);
}
+#[cfg(test)]
fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs: bool) {
// Tests that we're able to enforce HTLCs onchain during the different stages of a splice.
let chanmon_cfgs = create_chanmon_cfgs(2);
@@ -926,6 +931,7 @@ fn test_splice_reestablish() {
do_test_splice_reestablish(true, true);
}
+#[cfg(test)]
fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) {
// Test that we're able to reestablish the channel succesfully throughout the lifecycle of a splice.
let chanmon_cfgs = create_chanmon_cfgs(2);
@@ -1188,6 +1194,7 @@ fn test_propose_splice_while_disconnected() {
do_test_propose_splice_while_disconnected(true, true);
}
+#[cfg(test)]
fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) {
// Test that both nodes are able to propose a splice while the counterparty is disconnected, and
// whoever doesn't go first due to the quiescence tie-breaker, will retry their splice after the
@@ -1804,6 +1811,7 @@ fn fail_quiescent_action_on_channel_close() {
check_added_monitors(&nodes[0], 1);
}
+#[cfg(test)]
fn do_test_splice_with_inflight_htlc_forward_and_resolution(expire_scid_pre_forward: bool) {
// Test that we are still able to forward and resolve HTLCs while the original SCIDs contained
// in the onion packets have now changed due channel splices becoming locked.
Why this scored 44/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.