0.1 Backports for 0.1.2

.github/workflows/semver.yml

@@ -0,0 +1,57 @@
+name: SemVer checks
+on:
+ push:
+ branches-ignore:
+ - master
+ pull_request:
+ branches-ignore:
+ - master
+
+jobs:
+ semver-checks:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout source code
+ uses: actions/checkout@v4
+ - name: Install Rust stable toolchain
+ run: |
+ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
+ rustup override set stable
+ - name: Check SemVer with default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: default-features
+ - name: Check SemVer *without* default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: only-explicit-features
+ - name: Check lightning-background-processor SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-background-processor
+ feature-group: only-explicit-features
+ features: futures
+ - name: Check lightning-block-sync SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-block-sync
+ feature-group: only-explicit-features
+ features: rpc-client,rest-client
+ - name: Check lightning-transaction-sync electrum SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: electrum
+ - name: Check lightning-transaction-sync esplora-blocking SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-blocking
+ - name: Check lightning-transaction-sync esplora-async SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-async

lightning-background-processor/src/lib.rs

@@ -2379,8 +2379,8 @@ mod tests {
  42,
  53,
  features,
- $nodes[0].node.get_our_node_id(),
- $nodes[1].node.get_our_node_id(),
+ $nodes[0].node.get_our_node_id().into(),
+ $nodes[1].node.get_our_node_id().into(),
  )
  .expect("Failed to update channel from partial announcement");
  let original_graph_description = $nodes[0].network_graph.to_string();

lightning/src/chain/channelmonitor.rs

@@ -1509,8 +1509,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	fn provide_latest_holder_commitment_tx(
  &self, holder_commitment_tx: HolderCommitmentTransaction,
  htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
-	) -> Result<(), ()> {
- self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new()).map_err(|_| ())
+	) {
+ self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new())
 	}
 
 	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
@@ -1737,10 +1737,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
  self.inner.lock().unwrap().get_cur_holder_commitment_number()
 	}
 
-	/// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
-	/// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
-	pub(crate) fn offchain_closed(&self) -> bool {
- self.inner.lock().unwrap().lockdown_from_offchain
+	/// Fetches whether this monitor has marked the channel as closed and will refuse any further
+	/// updates to the commitment transactions.
+	///
+	/// It can be marked closed in a few different ways, including via a
+	/// [`ChannelMonitorUpdateStep::ChannelForceClosed`] or if the channel has been closed
+	/// on-chain.
+	pub(crate) fn no_further_updates_allowed(&self) -> bool {
+ self.inner.lock().unwrap().no_further_updates_allowed()
 	}
 
 	/// Gets the `node_id` of the counterparty for this channel.
@@ -2901,7 +2905,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// is important that any clones of this channel monitor (including remote clones) by kept
 	/// up-to-date as our holder commitment transaction is updated.
 	/// Panics if set_on_holder_tx_csv has never been called.
-	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) -> Result<(), &'static str> {
+	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) {
  if htlc_outputs.iter().any(|(_, s, _)| s.is_some()) {
  // If we have non-dust HTLCs in htlc_outputs, ensure they match the HTLCs in the
  // `holder_commitment_tx`. In the future, we'll no longer provide the redundant data
@@ -2978,10 +2982,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  self.counterparty_fulfilled_htlcs.insert(*claimed_htlc_id, *claimed_preimage);
  }
- if self.holder_tx_signed {
- return Err("Latest holder commitment signed has already been signed, update is rejected");
- }
- Ok(())
 	}
 
 	/// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
@@ -3202,11 +3202,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs, nondust_htlc_sources } => {
  log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
  if self.lockdown_from_offchain { panic!(); }
- if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone()) {
- log_error!(logger, "Providing latest holder commitment transaction failed/was refused:");
- log_error!(logger, " {}", e);
- ret = Err(());
- }
+ self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone());
  }
  ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => {
  log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -3286,12 +3282,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if ret.is_ok() && (self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed) && is_pre_close_update {
+ if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
  log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
  Err(())
  } else { ret }
 	}
 
+	fn no_further_updates_allowed(&self) -> bool {
+ self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
+	}
+
 	fn get_latest_update_id(&self) -> u64 {
  self.latest_update_id
 	}
@@ -3564,11 +3564,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  return (claimable_outpoints, to_counterparty_output_info);
  }
  let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
+ let counterparty_spendable_height = if htlc.offered {
+ htlc.cltv_expiry
+ } else {
+ height
+ };
  let justice_package = PackageTemplate::build_package(
  commitment_txid,
  transaction_output_index,
  PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
-htlc.cltv_expiry,
+counterparty_spendable_height,
  );
  claimable_outpoints.push(justice_package);
  }
@@ -3869,35 +3874,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
  }
- if self.holder_tx_signed {
- // If we've signed, we may have broadcast either commitment (prev or current), and
- // attempted to claim from it immediately without waiting for a confirmation.
- if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ // Cancel any pending claims for any holder commitments in case they had previously
+ // confirmed or been signed (in which case we will start attempting to claim without
+ // waiting for confirmation).
+ if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+ self.current_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+ if let Some(vout) = htlc.transaction_output_index {
+ outpoint.vout = vout;
+ self.onchain_tx_handler.abandon_claim(&outpoint);
+ }
+ }
+ }
+ if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
+ if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
  log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
-self.current_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+prev_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
  if let Some(vout) = htlc.transaction_output_index {
  outpoint.vout = vout;
  self.onchain_tx_handler.abandon_claim(&outpoint);
  }
  }
  }
- if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
- if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
- log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
- prev_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
- if let Some(vout) = htlc.transaction_output_index {
- outpoint.vout = vout;
- self.onchain_tx_handler.abandon_claim(&outpoint);
- }
- }
- }
- }
- } else {
- // No previous claim.
  }
 	}
 
@@ -4233,7 +4235,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if self.lockdown_from_offchain || self.funding_spend_seen || self.holder_tx_signed {
+ if self.no_further_updates_allowed() {
  // Fail back HTLCs on backwards channels if they expire within
  // `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a
  // point where no further off-chain updates will be accepted). If we haven't seen the
@@ -5384,7 +5386,7 @@ mod tests {
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
 
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()),
  preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()),
@@ -5422,7 +5424,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..5]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
  monitor.provide_secret(281474976710653, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12);
@@ -5433,7 +5435,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..3]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx,
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
  monitor.provide_secret(281474976710652, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5);

lightning/src/chain/package.rs

@@ -699,8 +699,13 @@ impl PackageSolvingData {
  match self {
  PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
- PackageSolvingData::RevokedHTLCOutput(..) =>
- PackageMalleability::Malleable(AggregationCluster::Pinnable),
+ PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
+ if htlc.offered {
+ PackageMalleability::Malleable(AggregationCluster::Unpinnable)
+ } else {
+ PackageMalleability::Malleable(AggregationCluster::Pinnable)
+ }
+ },
  PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
  PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
@@ -771,10 +776,12 @@ pub struct PackageTemplate {
 	/// Block height at which our counterparty can potentially claim this output as well (assuming
 	/// they have the keys or information required to do so).
 	///
-	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
-	/// because the counterparty can potentially spend it. It is also used internally by
-	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
-	/// type of output.
+	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
+	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
+	/// identify when an output must be claimed by, depending on the type of output.
+	///
+	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
+	/// we upgraded from LDK 0.1 or prior.
 	counterparty_spendable_height: u32,
 	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
 	// (either claimed output value or external utxo), it will keep increasing until holder
@@ -834,17 +841,17 @@ impl PackageTemplate {
  // Now check that we only merge packages if they are both unpinnable or both
  // pinnable.
  let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
- self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
- other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_pinnable && other_pinnable {
  return true;
  }
 
  let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
- self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
- other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_unpinnable && other_unpinnable {
  return true;
  }
@@ -855,13 +862,6 @@ impl PackageTemplate {
 	pub(crate) fn is_malleable(&self) -> bool {
  matches!(self.malleability, PackageMalleability::Malleable(..))
 	}
-	/// The height at which our counterparty may be able to spend this output.
-	///
-	/// This is an important limit for aggregation as after this height our counterparty may be
-	/// able to pin transactions spending this output in the mempool.
-	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
- self.counterparty_spendable_height
-	}
 	pub(crate) fn previous_feerate(&self) -> u64 {
  self.feerate_previous
 	}
@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
  (4, _height_original, option), // Written with a dummy value since 0.1
  (6, height_timer, option),
  });
+ for (_, input) in &inputs {
+ if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
+ // LDK versions through 0.1 set the wrong counterparty_spendable_height for
+ // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
+ // claim with a preimage immediately). Here we detect this and reset the value to
+ // zero, as the value is unused except for merging decisions which doesn't care
+ // about any values below the current height.
+ if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
+ counterparty_spendable_height = 0;
+ }
+ }
+ }
  Ok(PackageTemplate {
  inputs,
  malleability,

lightning/src/ln/channelmanager.rs

@@ -13364,8 +13364,8 @@ where
  // claim.
  // Note that a `ChannelMonitor` is created with `update_id` 0 and after we
  // provide it with a closure update its `update_id` will be at 1.
- if !monitor.offchain_closed() || monitor.get_latest_update_id() > 1 {
- should_queue_fc_update = !monitor.offchain_closed();
+ if !monitor.no_further_updates_allowed() || monitor.get_latest_update_id() > 1 {
+ should_queue_fc_update = !monitor.no_further_updates_allowed();
  let mut latest_update_id = monitor.get_latest_update_id();
  if should_queue_fc_update {
  latest_update_id += 1;

.github/workflows/semver.yml

@@ -0,0 +1,57 @@
+name: SemVer checks
+on:
+ push:
+ branches-ignore:
+ - master
+ pull_request:
+ branches-ignore:
+ - master
+
+jobs:
+ semver-checks:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout source code
+ uses: actions/checkout@v4
+ - name: Install Rust stable toolchain
+ run: |
+ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
+ rustup override set stable
+ - name: Check SemVer with default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: default-features
+ - name: Check SemVer *without* default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: only-explicit-features
+ - name: Check lightning-background-processor SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-background-processor
+ feature-group: only-explicit-features
+ features: futures
+ - name: Check lightning-block-sync SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-block-sync
+ feature-group: only-explicit-features
+ features: rpc-client,rest-client
+ - name: Check lightning-transaction-sync electrum SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: electrum
+ - name: Check lightning-transaction-sync esplora-blocking SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-blocking
+ - name: Check lightning-transaction-sync esplora-async SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-async

lightning-background-processor/src/lib.rs

@@ -2379,8 +2379,8 @@ mod tests {
  42,
  53,
  features,
- $nodes[0].node.get_our_node_id(),
- $nodes[1].node.get_our_node_id(),
+ $nodes[0].node.get_our_node_id().into(),
+ $nodes[1].node.get_our_node_id().into(),
  )
  .expect("Failed to update channel from partial announcement");
  let original_graph_description = $nodes[0].network_graph.to_string();

lightning/src/chain/channelmonitor.rs

@@ -1509,8 +1509,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	fn provide_latest_holder_commitment_tx(
  &self, holder_commitment_tx: HolderCommitmentTransaction,
  htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
-	) -> Result<(), ()> {
- self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new()).map_err(|_| ())
+	) {
+ self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new())
 	}
 
 	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
@@ -1737,10 +1737,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
  self.inner.lock().unwrap().get_cur_holder_commitment_number()
 	}
 
-	/// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
-	/// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
-	pub(crate) fn offchain_closed(&self) -> bool {
- self.inner.lock().unwrap().lockdown_from_offchain
+	/// Fetches whether this monitor has marked the channel as closed and will refuse any further
+	/// updates to the commitment transactions.
+	///
+	/// It can be marked closed in a few different ways, including via a
+	/// [`ChannelMonitorUpdateStep::ChannelForceClosed`] or if the channel has been closed
+	/// on-chain.
+	pub(crate) fn no_further_updates_allowed(&self) -> bool {
+ self.inner.lock().unwrap().no_further_updates_allowed()
 	}
 
 	/// Gets the `node_id` of the counterparty for this channel.
@@ -2901,7 +2905,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// is important that any clones of this channel monitor (including remote clones) by kept
 	/// up-to-date as our holder commitment transaction is updated.
 	/// Panics if set_on_holder_tx_csv has never been called.
-	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) -> Result<(), &'static str> {
+	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) {
  if htlc_outputs.iter().any(|(_, s, _)| s.is_some()) {
  // If we have non-dust HTLCs in htlc_outputs, ensure they match the HTLCs in the
  // `holder_commitment_tx`. In the future, we'll no longer provide the redundant data
@@ -2978,10 +2982,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  self.counterparty_fulfilled_htlcs.insert(*claimed_htlc_id, *claimed_preimage);
  }
- if self.holder_tx_signed {
- return Err("Latest holder commitment signed has already been signed, update is rejected");
- }
- Ok(())
 	}
 
 	/// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
@@ -3202,11 +3202,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs, nondust_htlc_sources } => {
  log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
  if self.lockdown_from_offchain { panic!(); }
- if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone()) {
- log_error!(logger, "Providing latest holder commitment transaction failed/was refused:");
- log_error!(logger, " {}", e);
- ret = Err(());
- }
+ self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone());
  }
  ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => {
  log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -3286,12 +3282,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if ret.is_ok() && (self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed) && is_pre_close_update {
+ if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
  log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
  Err(())
  } else { ret }
 	}
 
+	fn no_further_updates_allowed(&self) -> bool {
+ self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
+	}
+
 	fn get_latest_update_id(&self) -> u64 {
  self.latest_update_id
 	}
@@ -3564,11 +3564,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  return (claimable_outpoints, to_counterparty_output_info);
  }
  let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
+ let counterparty_spendable_height = if htlc.offered {
+ htlc.cltv_expiry
+ } else {
+ height
+ };
  let justice_package = PackageTemplate::build_package(
  commitment_txid,
  transaction_output_index,
  PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
-htlc.cltv_expiry,
+counterparty_spendable_height,
  );
  claimable_outpoints.push(justice_package);
  }
@@ -3869,35 +3874,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
  }
- if self.holder_tx_signed {
- // If we've signed, we may have broadcast either commitment (prev or current), and
- // attempted to claim from it immediately without waiting for a confirmation.
- if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ // Cancel any pending claims for any holder commitments in case they had previously
+ // confirmed or been signed (in which case we will start attempting to claim without
+ // waiting for confirmation).
+ if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+ self.current_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+ if let Some(vout) = htlc.transaction_output_index {
+ outpoint.vout = vout;
+ self.onchain_tx_handler.abandon_claim(&outpoint);
+ }
+ }
+ }
+ if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
+ if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
  log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
-self.current_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+prev_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
  if let Some(vout) = htlc.transaction_output_index {
  outpoint.vout = vout;
  self.onchain_tx_handler.abandon_claim(&outpoint);
  }
  }
  }
- if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
- if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
- log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
- prev_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
- if let Some(vout) = htlc.transaction_output_index {
- outpoint.vout = vout;
- self.onchain_tx_handler.abandon_claim(&outpoint);
- }
- }
- }
- }
- } else {
- // No previous claim.
  }
 	}
 
@@ -4233,7 +4235,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if self.lockdown_from_offchain || self.funding_spend_seen || self.holder_tx_signed {
+ if self.no_further_updates_allowed() {
  // Fail back HTLCs on backwards channels if they expire within
  // `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a
  // point where no further off-chain updates will be accepted). If we haven't seen the
@@ -5384,7 +5386,7 @@ mod tests {
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
 
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()),
  preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()),
@@ -5422,7 +5424,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..5]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
  monitor.provide_secret(281474976710653, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12);
@@ -5433,7 +5435,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..3]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx,
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
  monitor.provide_secret(281474976710652, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5);

lightning/src/chain/package.rs

@@ -699,8 +699,13 @@ impl PackageSolvingData {
  match self {
  PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
- PackageSolvingData::RevokedHTLCOutput(..) =>
- PackageMalleability::Malleable(AggregationCluster::Pinnable),
+ PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
+ if htlc.offered {
+ PackageMalleability::Malleable(AggregationCluster::Unpinnable)
+ } else {
+ PackageMalleability::Malleable(AggregationCluster::Pinnable)
+ }
+ },
  PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
  PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
@@ -771,10 +776,12 @@ pub struct PackageTemplate {
 	/// Block height at which our counterparty can potentially claim this output as well (assuming
 	/// they have the keys or information required to do so).
 	///
-	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
-	/// because the counterparty can potentially spend it. It is also used internally by
-	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
-	/// type of output.
+	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
+	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
+	/// identify when an output must be claimed by, depending on the type of output.
+	///
+	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
+	/// we upgraded from LDK 0.1 or prior.
 	counterparty_spendable_height: u32,
 	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
 	// (either claimed output value or external utxo), it will keep increasing until holder
@@ -834,17 +841,17 @@ impl PackageTemplate {
  // Now check that we only merge packages if they are both unpinnable or both
  // pinnable.
  let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
- self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
- other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_pinnable && other_pinnable {
  return true;
  }
 
  let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
- self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
- other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_unpinnable && other_unpinnable {
  return true;
  }
@@ -855,13 +862,6 @@ impl PackageTemplate {
 	pub(crate) fn is_malleable(&self) -> bool {
  matches!(self.malleability, PackageMalleability::Malleable(..))
 	}
-	/// The height at which our counterparty may be able to spend this output.
-	///
-	/// This is an important limit for aggregation as after this height our counterparty may be
-	/// able to pin transactions spending this output in the mempool.
-	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
- self.counterparty_spendable_height
-	}
 	pub(crate) fn previous_feerate(&self) -> u64 {
  self.feerate_previous
 	}
@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
  (4, _height_original, option), // Written with a dummy value since 0.1
  (6, height_timer, option),
  });
+ for (_, input) in &inputs {
+ if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
+ // LDK versions through 0.1 set the wrong counterparty_spendable_height for
+ // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
+ // claim with a preimage immediately). Here we detect this and reset the value to
+ // zero, as the value is unused except for merging decisions which doesn't care
+ // about any values below the current height.
+ if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
+ counterparty_spendable_height = 0;
+ }
+ }
+ }
  Ok(PackageTemplate {
  inputs,
  malleability,

lightning/src/ln/channelmanager.rs

@@ -13364,8 +13364,8 @@ where
  // claim.
  // Note that a `ChannelMonitor` is created with `update_id` 0 and after we
  // provide it with a closure update its `update_id` will be at 1.
- if !monitor.offchain_closed() || monitor.get_latest_update_id() > 1 {
- should_queue_fc_update = !monitor.offchain_closed();
+ if !monitor.no_further_updates_allowed() || monitor.get_latest_update_id() > 1 {
+ should_queue_fc_update = !monitor.no_further_updates_allowed();
  let mut latest_update_id = monitor.get_latest_update_id();
  if should_queue_fc_update {
  latest_update_id += 1;

.github/workflows/semver.yml

@@ -0,0 +1,57 @@
+name: SemVer checks
+on:
+ push:
+ branches-ignore:
+ - master
+ pull_request:
+ branches-ignore:
+ - master
+
+jobs:
+ semver-checks:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout source code
+ uses: actions/checkout@v4
+ - name: Install Rust stable toolchain
+ run: |
+ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
+ rustup override set stable
+ - name: Check SemVer with default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: default-features
+ - name: Check SemVer *without* default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: only-explicit-features
+ - name: Check lightning-background-processor SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-background-processor
+ feature-group: only-explicit-features
+ features: futures
+ - name: Check lightning-block-sync SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-block-sync
+ feature-group: only-explicit-features
+ features: rpc-client,rest-client
+ - name: Check lightning-transaction-sync electrum SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: electrum
+ - name: Check lightning-transaction-sync esplora-blocking SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-blocking
+ - name: Check lightning-transaction-sync esplora-async SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-async

lightning-background-processor/src/lib.rs

@@ -2379,8 +2379,8 @@ mod tests {
  42,
  53,
  features,
- $nodes[0].node.get_our_node_id(),
- $nodes[1].node.get_our_node_id(),
+ $nodes[0].node.get_our_node_id().into(),
+ $nodes[1].node.get_our_node_id().into(),
  )
  .expect("Failed to update channel from partial announcement");
  let original_graph_description = $nodes[0].network_graph.to_string();

lightning/src/chain/channelmonitor.rs

@@ -1509,8 +1509,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	fn provide_latest_holder_commitment_tx(
  &self, holder_commitment_tx: HolderCommitmentTransaction,
  htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
-	) -> Result<(), ()> {
- self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new()).map_err(|_| ())
+	) {
+ self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new())
 	}
 
 	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
@@ -1737,10 +1737,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
  self.inner.lock().unwrap().get_cur_holder_commitment_number()
 	}
 
-	/// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
-	/// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
-	pub(crate) fn offchain_closed(&self) -> bool {
- self.inner.lock().unwrap().lockdown_from_offchain
+	/// Fetches whether this monitor has marked the channel as closed and will refuse any further
+	/// updates to the commitment transactions.
+	///
+	/// It can be marked closed in a few different ways, including via a
+	/// [`ChannelMonitorUpdateStep::ChannelForceClosed`] or if the channel has been closed
+	/// on-chain.
+	pub(crate) fn no_further_updates_allowed(&self) -> bool {
+ self.inner.lock().unwrap().no_further_updates_allowed()
 	}
 
 	/// Gets the `node_id` of the counterparty for this channel.
@@ -2901,7 +2905,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// is important that any clones of this channel monitor (including remote clones) by kept
 	/// up-to-date as our holder commitment transaction is updated.
 	/// Panics if set_on_holder_tx_csv has never been called.
-	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) -> Result<(), &'static str> {
+	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) {
  if htlc_outputs.iter().any(|(_, s, _)| s.is_some()) {
  // If we have non-dust HTLCs in htlc_outputs, ensure they match the HTLCs in the
  // `holder_commitment_tx`. In the future, we'll no longer provide the redundant data
@@ -2978,10 +2982,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  self.counterparty_fulfilled_htlcs.insert(*claimed_htlc_id, *claimed_preimage);
  }
- if self.holder_tx_signed {
- return Err("Latest holder commitment signed has already been signed, update is rejected");
- }
- Ok(())
 	}
 
 	/// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
@@ -3202,11 +3202,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs, nondust_htlc_sources } => {
  log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
  if self.lockdown_from_offchain { panic!(); }
- if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone()) {
- log_error!(logger, "Providing latest holder commitment transaction failed/was refused:");
- log_error!(logger, " {}", e);
- ret = Err(());
- }
+ self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone());
  }
  ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => {
  log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -3286,12 +3282,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if ret.is_ok() && (self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed) && is_pre_close_update {
+ if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
  log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
  Err(())
  } else { ret }
 	}
 
+	fn no_further_updates_allowed(&self) -> bool {
+ self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
+	}
+
 	fn get_latest_update_id(&self) -> u64 {
  self.latest_update_id
 	}
@@ -3564,11 +3564,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  return (claimable_outpoints, to_counterparty_output_info);
  }
  let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
+ let counterparty_spendable_height = if htlc.offered {
+ htlc.cltv_expiry
+ } else {
+ height
+ };
  let justice_package = PackageTemplate::build_package(
  commitment_txid,
  transaction_output_index,
  PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
-htlc.cltv_expiry,
+counterparty_spendable_height,
  );
  claimable_outpoints.push(justice_package);
  }
@@ -3869,35 +3874,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
  }
- if self.holder_tx_signed {
- // If we've signed, we may have broadcast either commitment (prev or current), and
- // attempted to claim from it immediately without waiting for a confirmation.
- if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ // Cancel any pending claims for any holder commitments in case they had previously
+ // confirmed or been signed (in which case we will start attempting to claim without
+ // waiting for confirmation).
+ if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+ self.current_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+ if let Some(vout) = htlc.transaction_output_index {
+ outpoint.vout = vout;
+ self.onchain_tx_handler.abandon_claim(&outpoint);
+ }
+ }
+ }
+ if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
+ if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
  log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
-self.current_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+prev_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
  if let Some(vout) = htlc.transaction_output_index {
  outpoint.vout = vout;
  self.onchain_tx_handler.abandon_claim(&outpoint);
  }
  }
  }
- if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
- if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
- log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
- prev_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
- if let Some(vout) = htlc.transaction_output_index {
- outpoint.vout = vout;
- self.onchain_tx_handler.abandon_claim(&outpoint);
- }
- }
- }
- }
- } else {
- // No previous claim.
  }
 	}
 
@@ -4233,7 +4235,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if self.lockdown_from_offchain || self.funding_spend_seen || self.holder_tx_signed {
+ if self.no_further_updates_allowed() {
  // Fail back HTLCs on backwards channels if they expire within
  // `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a
  // point where no further off-chain updates will be accepted). If we haven't seen the
@@ -5384,7 +5386,7 @@ mod tests {
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
 
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()),
  preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()),
@@ -5422,7 +5424,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..5]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
  monitor.provide_secret(281474976710653, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12);
@@ -5433,7 +5435,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..3]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx,
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
  monitor.provide_secret(281474976710652, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5);

lightning/src/chain/package.rs

@@ -699,8 +699,13 @@ impl PackageSolvingData {
  match self {
  PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
- PackageSolvingData::RevokedHTLCOutput(..) =>
- PackageMalleability::Malleable(AggregationCluster::Pinnable),
+ PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
+ if htlc.offered {
+ PackageMalleability::Malleable(AggregationCluster::Unpinnable)
+ } else {
+ PackageMalleability::Malleable(AggregationCluster::Pinnable)
+ }
+ },
  PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
  PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
@@ -771,10 +776,12 @@ pub struct PackageTemplate {
 	/// Block height at which our counterparty can potentially claim this output as well (assuming
 	/// they have the keys or information required to do so).
 	///
-	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
-	/// because the counterparty can potentially spend it. It is also used internally by
-	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
-	/// type of output.
+	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
+	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
+	/// identify when an output must be claimed by, depending on the type of output.
+	///
+	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
+	/// we upgraded from LDK 0.1 or prior.
 	counterparty_spendable_height: u32,
 	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
 	// (either claimed output value or external utxo), it will keep increasing until holder
@@ -834,17 +841,17 @@ impl PackageTemplate {
  // Now check that we only merge packages if they are both unpinnable or both
  // pinnable.
  let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
- self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
- other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_pinnable && other_pinnable {
  return true;
  }
 
  let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
- self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
- other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_unpinnable && other_unpinnable {
  return true;
  }
@@ -855,13 +862,6 @@ impl PackageTemplate {
 	pub(crate) fn is_malleable(&self) -> bool {
  matches!(self.malleability, PackageMalleability::Malleable(..))
 	}
-	/// The height at which our counterparty may be able to spend this output.
-	///
-	/// This is an important limit for aggregation as after this height our counterparty may be
-	/// able to pin transactions spending this output in the mempool.
-	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
- self.counterparty_spendable_height
-	}
 	pub(crate) fn previous_feerate(&self) -> u64 {
  self.feerate_previous
 	}
@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
  (4, _height_original, option), // Written with a dummy value since 0.1
  (6, height_timer, option),
  });
+ for (_, input) in &inputs {
+ if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
+ // LDK versions through 0.1 set the wrong counterparty_spendable_height for
+ // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
+ // claim with a preimage immediately). Here we detect this and reset the value to
+ // zero, as the value is unused except for merging decisions which doesn't care
+ // about any values below the current height.
+ if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
+ counterparty_spendable_height = 0;
+ }
+ }
+ }
  Ok(PackageTemplate {
  inputs,
  malleability,

lightning/src/ln/channelmanager.rs

@@ -13364,8 +13364,8 @@ where
  // claim.
  // Note that a `ChannelMonitor` is created with `update_id` 0 and after we
  // provide it with a closure update its `update_id` will be at 1.
- if !monitor.offchain_closed() || monitor.get_latest_update_id() > 1 {
- should_queue_fc_update = !monitor.offchain_closed();
+ if !monitor.no_further_updates_allowed() || monitor.get_latest_update_id() > 1 {
+ should_queue_fc_update = !monitor.no_further_updates_allowed();
  let mut latest_update_id = monitor.get_latest_update_id();
  if should_queue_fc_update {
  latest_update_id += 1;

.github/workflows/semver.yml

@@ -0,0 +1,57 @@
+name: SemVer checks
+on:
+ push:
+ branches-ignore:
+ - master
+ pull_request:
+ branches-ignore:
+ - master
+
+jobs:
+ semver-checks:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout source code
+ uses: actions/checkout@v4
+ - name: Install Rust stable toolchain
+ run: |
+ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
+ rustup override set stable
+ - name: Check SemVer with default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: default-features
+ - name: Check SemVer *without* default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: only-explicit-features
+ - name: Check lightning-background-processor SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-background-processor
+ feature-group: only-explicit-features
+ features: futures
+ - name: Check lightning-block-sync SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-block-sync
+ feature-group: only-explicit-features
+ features: rpc-client,rest-client
+ - name: Check lightning-transaction-sync electrum SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: electrum
+ - name: Check lightning-transaction-sync esplora-blocking SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-blocking
+ - name: Check lightning-transaction-sync esplora-async SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-async

lightning-background-processor/src/lib.rs

@@ -2379,8 +2379,8 @@ mod tests {
  42,
  53,
  features,
- $nodes[0].node.get_our_node_id(),
- $nodes[1].node.get_our_node_id(),
+ $nodes[0].node.get_our_node_id().into(),
+ $nodes[1].node.get_our_node_id().into(),
  )
  .expect("Failed to update channel from partial announcement");
  let original_graph_description = $nodes[0].network_graph.to_string();

lightning/src/chain/channelmonitor.rs

@@ -1509,8 +1509,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	fn provide_latest_holder_commitment_tx(
  &self, holder_commitment_tx: HolderCommitmentTransaction,
  htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
-	) -> Result<(), ()> {
- self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new()).map_err(|_| ())
+	) {
+ self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new())
 	}
 
 	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
@@ -1737,10 +1737,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
  self.inner.lock().unwrap().get_cur_holder_commitment_number()
 	}
 
-	/// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
-	/// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
-	pub(crate) fn offchain_closed(&self) -> bool {
- self.inner.lock().unwrap().lockdown_from_offchain
+	/// Fetches whether this monitor has marked the channel as closed and will refuse any further
+	/// updates to the commitment transactions.
+	///
+	/// It can be marked closed in a few different ways, including via a
+	/// [`ChannelMonitorUpdateStep::ChannelForceClosed`] or if the channel has been closed
+	/// on-chain.
+	pub(crate) fn no_further_updates_allowed(&self) -> bool {
+ self.inner.lock().unwrap().no_further_updates_allowed()
 	}
 
 	/// Gets the `node_id` of the counterparty for this channel.
@@ -2901,7 +2905,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// is important that any clones of this channel monitor (including remote clones) by kept
 	/// up-to-date as our holder commitment transaction is updated.
 	/// Panics if set_on_holder_tx_csv has never been called.
-	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) -> Result<(), &'static str> {
+	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) {
  if htlc_outputs.iter().any(|(_, s, _)| s.is_some()) {
  // If we have non-dust HTLCs in htlc_outputs, ensure they match the HTLCs in the
  // `holder_commitment_tx`. In the future, we'll no longer provide the redundant data
@@ -2978,10 +2982,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  self.counterparty_fulfilled_htlcs.insert(*claimed_htlc_id, *claimed_preimage);
  }
- if self.holder_tx_signed {
- return Err("Latest holder commitment signed has already been signed, update is rejected");
- }
- Ok(())
 	}
 
 	/// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
@@ -3202,11 +3202,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs, nondust_htlc_sources } => {
  log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
  if self.lockdown_from_offchain { panic!(); }
- if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone()) {
- log_error!(logger, "Providing latest holder commitment transaction failed/was refused:");
- log_error!(logger, " {}", e);
- ret = Err(());
- }
+ self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone());
  }
  ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => {
  log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -3286,12 +3282,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if ret.is_ok() && (self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed) && is_pre_close_update {
+ if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
  log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
  Err(())
  } else { ret }
 	}
 
+	fn no_further_updates_allowed(&self) -> bool {
+ self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
+	}
+
 	fn get_latest_update_id(&self) -> u64 {
  self.latest_update_id
 	}
@@ -3564,11 +3564,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  return (claimable_outpoints, to_counterparty_output_info);
  }
  let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
+ let counterparty_spendable_height = if htlc.offered {
+ htlc.cltv_expiry
+ } else {
+ height
+ };
  let justice_package = PackageTemplate::build_package(
  commitment_txid,
  transaction_output_index,
  PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
-htlc.cltv_expiry,
+counterparty_spendable_height,
  );
  claimable_outpoints.push(justice_package);
  }
@@ -3869,35 +3874,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
  }
- if self.holder_tx_signed {
- // If we've signed, we may have broadcast either commitment (prev or current), and
- // attempted to claim from it immediately without waiting for a confirmation.
- if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ // Cancel any pending claims for any holder commitments in case they had previously
+ // confirmed or been signed (in which case we will start attempting to claim without
+ // waiting for confirmation).
+ if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+ self.current_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+ if let Some(vout) = htlc.transaction_output_index {
+ outpoint.vout = vout;
+ self.onchain_tx_handler.abandon_claim(&outpoint);
+ }
+ }
+ }
+ if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
+ if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
  log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
-self.current_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+prev_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
  if let Some(vout) = htlc.transaction_output_index {
  outpoint.vout = vout;
  self.onchain_tx_handler.abandon_claim(&outpoint);
  }
  }
  }
- if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
- if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
- log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
- prev_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
- if let Some(vout) = htlc.transaction_output_index {
- outpoint.vout = vout;
- self.onchain_tx_handler.abandon_claim(&outpoint);
- }
- }
- }
- }
- } else {
- // No previous claim.
  }
 	}
 
@@ -4233,7 +4235,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if self.lockdown_from_offchain || self.funding_spend_seen || self.holder_tx_signed {
+ if self.no_further_updates_allowed() {
  // Fail back HTLCs on backwards channels if they expire within
  // `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a
  // point where no further off-chain updates will be accepted). If we haven't seen the
@@ -5384,7 +5386,7 @@ mod tests {
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
 
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()),
  preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()),
@@ -5422,7 +5424,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..5]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
  monitor.provide_secret(281474976710653, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12);
@@ -5433,7 +5435,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..3]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx,
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
  monitor.provide_secret(281474976710652, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5);

lightning/src/chain/package.rs

@@ -699,8 +699,13 @@ impl PackageSolvingData {
  match self {
  PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
- PackageSolvingData::RevokedHTLCOutput(..) =>
- PackageMalleability::Malleable(AggregationCluster::Pinnable),
+ PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
+ if htlc.offered {
+ PackageMalleability::Malleable(AggregationCluster::Unpinnable)
+ } else {
+ PackageMalleability::Malleable(AggregationCluster::Pinnable)
+ }
+ },
  PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
  PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
@@ -771,10 +776,12 @@ pub struct PackageTemplate {
 	/// Block height at which our counterparty can potentially claim this output as well (assuming
 	/// they have the keys or information required to do so).
 	///
-	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
-	/// because the counterparty can potentially spend it. It is also used internally by
-	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
-	/// type of output.
+	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
+	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
+	/// identify when an output must be claimed by, depending on the type of output.
+	///
+	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
+	/// we upgraded from LDK 0.1 or prior.
 	counterparty_spendable_height: u32,
 	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
 	// (either claimed output value or external utxo), it will keep increasing until holder
@@ -834,17 +841,17 @@ impl PackageTemplate {
  // Now check that we only merge packages if they are both unpinnable or both
  // pinnable.
  let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
- self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
- other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_pinnable && other_pinnable {
  return true;
  }
 
  let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
- self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
- other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_unpinnable && other_unpinnable {
  return true;
  }
@@ -855,13 +862,6 @@ impl PackageTemplate {
 	pub(crate) fn is_malleable(&self) -> bool {
  matches!(self.malleability, PackageMalleability::Malleable(..))
 	}
-	/// The height at which our counterparty may be able to spend this output.
-	///
-	/// This is an important limit for aggregation as after this height our counterparty may be
-	/// able to pin transactions spending this output in the mempool.
-	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
- self.counterparty_spendable_height
-	}
 	pub(crate) fn previous_feerate(&self) -> u64 {
  self.feerate_previous
 	}
@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
  (4, _height_original, option), // Written with a dummy value since 0.1
  (6, height_timer, option),
  });
+ for (_, input) in &inputs {
+ if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
+ // LDK versions through 0.1 set the wrong counterparty_spendable_height for
+ // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
+ // claim with a preimage immediately). Here we detect this and reset the value to
+ // zero, as the value is unused except for merging decisions which doesn't care
+ // about any values below the current height.
+ if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
+ counterparty_spendable_height = 0;
+ }
+ }
+ }
  Ok(PackageTemplate {
  inputs,
  malleability,

lightning/src/ln/channelmanager.rs

@@ -13364,8 +13364,8 @@ where
  // claim.
  // Note that a `ChannelMonitor` is created with `update_id` 0 and after we
  // provide it with a closure update its `update_id` will be at 1.
- if !monitor.offchain_closed() || monitor.get_latest_update_id() > 1 {
- should_queue_fc_update = !monitor.offchain_closed();
+ if !monitor.no_further_updates_allowed() || monitor.get_latest_update_id() > 1 {
+ should_queue_fc_update = !monitor.no_further_updates_allowed();
  let mut latest_update_id = monitor.get_latest_update_id();
  if should_queue_fc_update {
  latest_update_id += 1;

.github/workflows/semver.yml

@@ -0,0 +1,57 @@
+name: SemVer checks
+on:
+ push:
+ branches-ignore:
+ - master
+ pull_request:
+ branches-ignore:
+ - master
+
+jobs:
+ semver-checks:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout source code
+ uses: actions/checkout@v4
+ - name: Install Rust stable toolchain
+ run: |
+ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
+ rustup override set stable
+ - name: Check SemVer with default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: default-features
+ - name: Check SemVer *without* default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: only-explicit-features
+ - name: Check lightning-background-processor SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-background-processor
+ feature-group: only-explicit-features
+ features: futures
+ - name: Check lightning-block-sync SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-block-sync
+ feature-group: only-explicit-features
+ features: rpc-client,rest-client
+ - name: Check lightning-transaction-sync electrum SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: electrum
+ - name: Check lightning-transaction-sync esplora-blocking SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-blocking
+ - name: Check lightning-transaction-sync esplora-async SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-async

lightning-background-processor/src/lib.rs

@@ -2379,8 +2379,8 @@ mod tests {
  42,
  53,
  features,
- $nodes[0].node.get_our_node_id(),
- $nodes[1].node.get_our_node_id(),
+ $nodes[0].node.get_our_node_id().into(),
+ $nodes[1].node.get_our_node_id().into(),
  )
  .expect("Failed to update channel from partial announcement");
  let original_graph_description = $nodes[0].network_graph.to_string();

lightning/src/chain/channelmonitor.rs

@@ -1509,8 +1509,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	fn provide_latest_holder_commitment_tx(
  &self, holder_commitment_tx: HolderCommitmentTransaction,
  htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
-	) -> Result<(), ()> {
- self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new()).map_err(|_| ())
+	) {
+ self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new())
 	}
 
 	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
@@ -1737,10 +1737,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
  self.inner.lock().unwrap().get_cur_holder_commitment_number()
 	}
 
-	/// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
-	/// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
-	pub(crate) fn offchain_closed(&self) -> bool {
- self.inner.lock().unwrap().lockdown_from_offchain
+	/// Fetches whether this monitor has marked the channel as closed and will refuse any further
+	/// updates to the commitment transactions.
+	///
+	/// It can be marked closed in a few different ways, including via a
+	/// [`ChannelMonitorUpdateStep::ChannelForceClosed`] or if the channel has been closed
+	/// on-chain.
+	pub(crate) fn no_further_updates_allowed(&self) -> bool {
+ self.inner.lock().unwrap().no_further_updates_allowed()
 	}
 
 	/// Gets the `node_id` of the counterparty for this channel.
@@ -2901,7 +2905,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// is important that any clones of this channel monitor (including remote clones) by kept
 	/// up-to-date as our holder commitment transaction is updated.
 	/// Panics if set_on_holder_tx_csv has never been called.
-	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) -> Result<(), &'static str> {
+	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) {
  if htlc_outputs.iter().any(|(_, s, _)| s.is_some()) {
  // If we have non-dust HTLCs in htlc_outputs, ensure they match the HTLCs in the
  // `holder_commitment_tx`. In the future, we'll no longer provide the redundant data
@@ -2978,10 +2982,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  self.counterparty_fulfilled_htlcs.insert(*claimed_htlc_id, *claimed_preimage);
  }
- if self.holder_tx_signed {
- return Err("Latest holder commitment signed has already been signed, update is rejected");
- }
- Ok(())
 	}
 
 	/// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
@@ -3202,11 +3202,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs, nondust_htlc_sources } => {
  log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
  if self.lockdown_from_offchain { panic!(); }
- if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone()) {
- log_error!(logger, "Providing latest holder commitment transaction failed/was refused:");
- log_error!(logger, " {}", e);
- ret = Err(());
- }
+ self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone());
  }
  ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => {
  log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -3286,12 +3282,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if ret.is_ok() && (self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed) && is_pre_close_update {
+ if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
  log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
  Err(())
  } else { ret }
 	}
 
+	fn no_further_updates_allowed(&self) -> bool {
+ self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
+	}
+
 	fn get_latest_update_id(&self) -> u64 {
  self.latest_update_id
 	}
@@ -3564,11 +3564,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  return (claimable_outpoints, to_counterparty_output_info);
  }
  let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
+ let counterparty_spendable_height = if htlc.offered {
+ htlc.cltv_expiry
+ } else {
+ height
+ };
  let justice_package = PackageTemplate::build_package(
  commitment_txid,
  transaction_output_index,
  PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
-htlc.cltv_expiry,
+counterparty_spendable_height,
  );
  claimable_outpoints.push(justice_package);
  }
@@ -3869,35 +3874,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
  }
- if self.holder_tx_signed {
- // If we've signed, we may have broadcast either commitment (prev or current), and
- // attempted to claim from it immediately without waiting for a confirmation.
- if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ // Cancel any pending claims for any holder commitments in case they had previously
+ // confirmed or been signed (in which case we will start attempting to claim without
+ // waiting for confirmation).
+ if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+ self.current_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+ if let Some(vout) = htlc.transaction_output_index {
+ outpoint.vout = vout;
+ self.onchain_tx_handler.abandon_claim(&outpoint);
+ }
+ }
+ }
+ if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
+ if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
  log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
-self.current_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+prev_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
  if let Some(vout) = htlc.transaction_output_index {
  outpoint.vout = vout;
  self.onchain_tx_handler.abandon_claim(&outpoint);
  }
  }
  }
- if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
- if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
- log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
- prev_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
- if let Some(vout) = htlc.transaction_output_index {
- outpoint.vout = vout;
- self.onchain_tx_handler.abandon_claim(&outpoint);
- }
- }
- }
- }
- } else {
- // No previous claim.
  }
 	}
 
@@ -4233,7 +4235,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if self.lockdown_from_offchain || self.funding_spend_seen || self.holder_tx_signed {
+ if self.no_further_updates_allowed() {
  // Fail back HTLCs on backwards channels if they expire within
  // `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a
  // point where no further off-chain updates will be accepted). If we haven't seen the
@@ -5384,7 +5386,7 @@ mod tests {
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
 
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()),
  preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()),
@@ -5422,7 +5424,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..5]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
  monitor.provide_secret(281474976710653, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12);
@@ -5433,7 +5435,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..3]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx,
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
  monitor.provide_secret(281474976710652, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5);

lightning/src/chain/package.rs

@@ -699,8 +699,13 @@ impl PackageSolvingData {
  match self {
  PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
- PackageSolvingData::RevokedHTLCOutput(..) =>
- PackageMalleability::Malleable(AggregationCluster::Pinnable),
+ PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
+ if htlc.offered {
+ PackageMalleability::Malleable(AggregationCluster::Unpinnable)
+ } else {
+ PackageMalleability::Malleable(AggregationCluster::Pinnable)
+ }
+ },
  PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
  PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
@@ -771,10 +776,12 @@ pub struct PackageTemplate {
 	/// Block height at which our counterparty can potentially claim this output as well (assuming
 	/// they have the keys or information required to do so).
 	///
-	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
-	/// because the counterparty can potentially spend it. It is also used internally by
-	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
-	/// type of output.
+	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
+	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
+	/// identify when an output must be claimed by, depending on the type of output.
+	///
+	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
+	/// we upgraded from LDK 0.1 or prior.
 	counterparty_spendable_height: u32,
 	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
 	// (either claimed output value or external utxo), it will keep increasing until holder
@@ -834,17 +841,17 @@ impl PackageTemplate {
  // Now check that we only merge packages if they are both unpinnable or both
  // pinnable.
  let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
- self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
- other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_pinnable && other_pinnable {
  return true;
  }
 
  let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
- self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
- other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_unpinnable && other_unpinnable {
  return true;
  }
@@ -855,13 +862,6 @@ impl PackageTemplate {
 	pub(crate) fn is_malleable(&self) -> bool {
  matches!(self.malleability, PackageMalleability::Malleable(..))
 	}
-	/// The height at which our counterparty may be able to spend this output.
-	///
-	/// This is an important limit for aggregation as after this height our counterparty may be
-	/// able to pin transactions spending this output in the mempool.
-	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
- self.counterparty_spendable_height
-	}
 	pub(crate) fn previous_feerate(&self) -> u64 {
  self.feerate_previous
 	}
@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
  (4, _height_original, option), // Written with a dummy value since 0.1
  (6, height_timer, option),
  });
+ for (_, input) in &inputs {
+ if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
+ // LDK versions through 0.1 set the wrong counterparty_spendable_height for
+ // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
+ // claim with a preimage immediately). Here we detect this and reset the value to
+ // zero, as the value is unused except for merging decisions which doesn't care
+ // about any values below the current height.
+ if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
+ counterparty_spendable_height = 0;
+ }
+ }
+ }
  Ok(PackageTemplate {
  inputs,
  malleability,

lightning/src/ln/channelmanager.rs

@@ -13364,8 +13364,8 @@ where
  // claim.
  // Note that a `ChannelMonitor` is created with `update_id` 0 and after we
  // provide it with a closure update its `update_id` will be at 1.
- if !monitor.offchain_closed() || monitor.get_latest_update_id() > 1 {
- should_queue_fc_update = !monitor.offchain_closed();
+ if !monitor.no_further_updates_allowed() || monitor.get_latest_update_id() > 1 {
+ should_queue_fc_update = !monitor.no_further_updates_allowed();
  let mut latest_update_id = monitor.get_latest_update_id();
  if should_queue_fc_update {
  latest_update_id += 1;

.github/workflows/semver.yml

@@ -0,0 +1,57 @@
+name: SemVer checks
+on:
+ push:
+ branches-ignore:
+ - master
+ pull_request:
+ branches-ignore:
+ - master
+
+jobs:
+ semver-checks:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout source code
+ uses: actions/checkout@v4
+ - name: Install Rust stable toolchain
+ run: |
+ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
+ rustup override set stable
+ - name: Check SemVer with default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: default-features
+ - name: Check SemVer *without* default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: only-explicit-features
+ - name: Check lightning-background-processor SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-background-processor
+ feature-group: only-explicit-features
+ features: futures
+ - name: Check lightning-block-sync SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-block-sync
+ feature-group: only-explicit-features
+ features: rpc-client,rest-client
+ - name: Check lightning-transaction-sync electrum SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: electrum
+ - name: Check lightning-transaction-sync esplora-blocking SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-blocking
+ - name: Check lightning-transaction-sync esplora-async SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-async

lightning-background-processor/src/lib.rs

@@ -2379,8 +2379,8 @@ mod tests {
  42,
  53,
  features,
- $nodes[0].node.get_our_node_id(),
- $nodes[1].node.get_our_node_id(),
+ $nodes[0].node.get_our_node_id().into(),
+ $nodes[1].node.get_our_node_id().into(),
  )
  .expect("Failed to update channel from partial announcement");
  let original_graph_description = $nodes[0].network_graph.to_string();

lightning/src/chain/channelmonitor.rs

@@ -1509,8 +1509,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	fn provide_latest_holder_commitment_tx(
  &self, holder_commitment_tx: HolderCommitmentTransaction,
  htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
-	) -> Result<(), ()> {
- self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new()).map_err(|_| ())
+	) {
+ self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new())
 	}
 
 	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
@@ -1737,10 +1737,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
  self.inner.lock().unwrap().get_cur_holder_commitment_number()
 	}
 
-	/// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
-	/// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
-	pub(crate) fn offchain_closed(&self) -> bool {
- self.inner.lock().unwrap().lockdown_from_offchain
+	/// Fetches whether this monitor has marked the channel as closed and will refuse any further
+	/// updates to the commitment transactions.
+	///
+	/// It can be marked closed in a few different ways, including via a
+	/// [`ChannelMonitorUpdateStep::ChannelForceClosed`] or if the channel has been closed
+	/// on-chain.
+	pub(crate) fn no_further_updates_allowed(&self) -> bool {
+ self.inner.lock().unwrap().no_further_updates_allowed()
 	}
 
 	/// Gets the `node_id` of the counterparty for this channel.
@@ -2901,7 +2905,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// is important that any clones of this channel monitor (including remote clones) by kept
 	/// up-to-date as our holder commitment transaction is updated.
 	/// Panics if set_on_holder_tx_csv has never been called.
-	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) -> Result<(), &'static str> {
+	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) {
  if htlc_outputs.iter().any(|(_, s, _)| s.is_some()) {
  // If we have non-dust HTLCs in htlc_outputs, ensure they match the HTLCs in the
  // `holder_commitment_tx`. In the future, we'll no longer provide the redundant data
@@ -2978,10 +2982,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  self.counterparty_fulfilled_htlcs.insert(*claimed_htlc_id, *claimed_preimage);
  }
- if self.holder_tx_signed {
- return Err("Latest holder commitment signed has already been signed, update is rejected");
- }
- Ok(())
 	}
 
 	/// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
@@ -3202,11 +3202,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs, nondust_htlc_sources } => {
  log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
  if self.lockdown_from_offchain { panic!(); }
- if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone()) {
- log_error!(logger, "Providing latest holder commitment transaction failed/was refused:");
- log_error!(logger, " {}", e);
- ret = Err(());
- }
+ self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone());
  }
  ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => {
  log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -3286,12 +3282,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if ret.is_ok() && (self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed) && is_pre_close_update {
+ if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
  log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
  Err(())
  } else { ret }
 	}
 
+	fn no_further_updates_allowed(&self) -> bool {
+ self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
+	}
+
 	fn get_latest_update_id(&self) -> u64 {
  self.latest_update_id
 	}
@@ -3564,11 +3564,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  return (claimable_outpoints, to_counterparty_output_info);
  }
  let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
+ let counterparty_spendable_height = if htlc.offered {
+ htlc.cltv_expiry
+ } else {
+ height
+ };
  let justice_package = PackageTemplate::build_package(
  commitment_txid,
  transaction_output_index,
  PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
-htlc.cltv_expiry,
+counterparty_spendable_height,
  );
  claimable_outpoints.push(justice_package);
  }
@@ -3869,35 +3874,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
  }
- if self.holder_tx_signed {
- // If we've signed, we may have broadcast either commitment (prev or current), and
- // attempted to claim from it immediately without waiting for a confirmation.
- if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ // Cancel any pending claims for any holder commitments in case they had previously
+ // confirmed or been signed (in which case we will start attempting to claim without
+ // waiting for confirmation).
+ if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+ self.current_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+ if let Some(vout) = htlc.transaction_output_index {
+ outpoint.vout = vout;
+ self.onchain_tx_handler.abandon_claim(&outpoint);
+ }
+ }
+ }
+ if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
+ if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
  log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
-self.current_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+prev_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
  if let Some(vout) = htlc.transaction_output_index {
  outpoint.vout = vout;
  self.onchain_tx_handler.abandon_claim(&outpoint);
  }
  }
  }
- if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
- if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
- log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
- prev_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
- if let Some(vout) = htlc.transaction_output_index {
- outpoint.vout = vout;
- self.onchain_tx_handler.abandon_claim(&outpoint);
- }
- }
- }
- }
- } else {
- // No previous claim.
  }
 	}
 
@@ -4233,7 +4235,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if self.lockdown_from_offchain || self.funding_spend_seen || self.holder_tx_signed {
+ if self.no_further_updates_allowed() {
  // Fail back HTLCs on backwards channels if they expire within
  // `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a
  // point where no further off-chain updates will be accepted). If we haven't seen the
@@ -5384,7 +5386,7 @@ mod tests {
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
 
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()),
  preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()),
@@ -5422,7 +5424,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..5]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
  monitor.provide_secret(281474976710653, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12);
@@ -5433,7 +5435,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..3]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx,
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
  monitor.provide_secret(281474976710652, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5);

lightning/src/chain/package.rs

@@ -699,8 +699,13 @@ impl PackageSolvingData {
  match self {
  PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
- PackageSolvingData::RevokedHTLCOutput(..) =>
- PackageMalleability::Malleable(AggregationCluster::Pinnable),
+ PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
+ if htlc.offered {
+ PackageMalleability::Malleable(AggregationCluster::Unpinnable)
+ } else {
+ PackageMalleability::Malleable(AggregationCluster::Pinnable)
+ }
+ },
  PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
  PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
@@ -771,10 +776,12 @@ pub struct PackageTemplate {
 	/// Block height at which our counterparty can potentially claim this output as well (assuming
 	/// they have the keys or information required to do so).
 	///
-	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
-	/// because the counterparty can potentially spend it. It is also used internally by
-	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
-	/// type of output.
+	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
+	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
+	/// identify when an output must be claimed by, depending on the type of output.
+	///
+	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
+	/// we upgraded from LDK 0.1 or prior.
 	counterparty_spendable_height: u32,
 	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
 	// (either claimed output value or external utxo), it will keep increasing until holder
@@ -834,17 +841,17 @@ impl PackageTemplate {
  // Now check that we only merge packages if they are both unpinnable or both
  // pinnable.
  let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
- self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
- other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_pinnable && other_pinnable {
  return true;
  }
 
  let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
- self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
- other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_unpinnable && other_unpinnable {
  return true;
  }
@@ -855,13 +862,6 @@ impl PackageTemplate {
 	pub(crate) fn is_malleable(&self) -> bool {
  matches!(self.malleability, PackageMalleability::Malleable(..))
 	}
-	/// The height at which our counterparty may be able to spend this output.
-	///
-	/// This is an important limit for aggregation as after this height our counterparty may be
-	/// able to pin transactions spending this output in the mempool.
-	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
- self.counterparty_spendable_height
-	}
 	pub(crate) fn previous_feerate(&self) -> u64 {
  self.feerate_previous
 	}
@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
  (4, _height_original, option), // Written with a dummy value since 0.1
  (6, height_timer, option),
  });
+ for (_, input) in &inputs {
+ if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
+ // LDK versions through 0.1 set the wrong counterparty_spendable_height for
+ // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
+ // claim with a preimage immediately). Here we detect this and reset the value to
+ // zero, as the value is unused except for merging decisions which doesn't care
+ // about any values below the current height.
+ if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
+ counterparty_spendable_height = 0;
+ }
+ }
+ }
  Ok(PackageTemplate {
  inputs,
  malleability,

lightning/src/ln/channelmanager.rs

@@ -13364,8 +13364,8 @@ where
  // claim.
  // Note that a `ChannelMonitor` is created with `update_id` 0 and after we
  // provide it with a closure update its `update_id` will be at 1.
- if !monitor.offchain_closed() || monitor.get_latest_update_id() > 1 {
- should_queue_fc_update = !monitor.offchain_closed();
+ if !monitor.no_further_updates_allowed() || monitor.get_latest_update_id() > 1 {
+ should_queue_fc_update = !monitor.no_further_updates_allowed();
  let mut latest_update_id = monitor.get_latest_update_id();
  if should_queue_fc_update {
  latest_update_id += 1;

.github/workflows/semver.yml

@@ -0,0 +1,57 @@
+name: SemVer checks
+on:
+ push:
+ branches-ignore:
+ - master
+ pull_request:
+ branches-ignore:
+ - master
+
+jobs:
+ semver-checks:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout source code
+ uses: actions/checkout@v4
+ - name: Install Rust stable toolchain
+ run: |
+ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
+ rustup override set stable
+ - name: Check SemVer with default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: default-features
+ - name: Check SemVer *without* default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: only-explicit-features
+ - name: Check lightning-background-processor SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-background-processor
+ feature-group: only-explicit-features
+ features: futures
+ - name: Check lightning-block-sync SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-block-sync
+ feature-group: only-explicit-features
+ features: rpc-client,rest-client
+ - name: Check lightning-transaction-sync electrum SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: electrum
+ - name: Check lightning-transaction-sync esplora-blocking SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-blocking
+ - name: Check lightning-transaction-sync esplora-async SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-async

lightning-background-processor/src/lib.rs

@@ -2379,8 +2379,8 @@ mod tests {
  42,
  53,
  features,
- $nodes[0].node.get_our_node_id(),
- $nodes[1].node.get_our_node_id(),
+ $nodes[0].node.get_our_node_id().into(),
+ $nodes[1].node.get_our_node_id().into(),
  )
  .expect("Failed to update channel from partial announcement");
  let original_graph_description = $nodes[0].network_graph.to_string();

lightning/src/chain/channelmonitor.rs

@@ -1509,8 +1509,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	fn provide_latest_holder_commitment_tx(
  &self, holder_commitment_tx: HolderCommitmentTransaction,
  htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
-	) -> Result<(), ()> {
- self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new()).map_err(|_| ())
+	) {
+ self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new())
 	}
 
 	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
@@ -1737,10 +1737,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
  self.inner.lock().unwrap().get_cur_holder_commitment_number()
 	}
 
-	/// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
-	/// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
-	pub(crate) fn offchain_closed(&self) -> bool {
- self.inner.lock().unwrap().lockdown_from_offchain
+	/// Fetches whether this monitor has marked the channel as closed and will refuse any further
+	/// updates to the commitment transactions.
+	///
+	/// It can be marked closed in a few different ways, including via a
+	/// [`ChannelMonitorUpdateStep::ChannelForceClosed`] or if the channel has been closed
+	/// on-chain.
+	pub(crate) fn no_further_updates_allowed(&self) -> bool {
+ self.inner.lock().unwrap().no_further_updates_allowed()
 	}
 
 	/// Gets the `node_id` of the counterparty for this channel.
@@ -2901,7 +2905,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// is important that any clones of this channel monitor (including remote clones) by kept
 	/// up-to-date as our holder commitment transaction is updated.
 	/// Panics if set_on_holder_tx_csv has never been called.
-	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) -> Result<(), &'static str> {
+	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) {
  if htlc_outputs.iter().any(|(_, s, _)| s.is_some()) {
  // If we have non-dust HTLCs in htlc_outputs, ensure they match the HTLCs in the
  // `holder_commitment_tx`. In the future, we'll no longer provide the redundant data
@@ -2978,10 +2982,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  self.counterparty_fulfilled_htlcs.insert(*claimed_htlc_id, *claimed_preimage);
  }
- if self.holder_tx_signed {
- return Err("Latest holder commitment signed has already been signed, update is rejected");
- }
- Ok(())
 	}
 
 	/// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
@@ -3202,11 +3202,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs, nondust_htlc_sources } => {
  log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
  if self.lockdown_from_offchain { panic!(); }
- if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone()) {
- log_error!(logger, "Providing latest holder commitment transaction failed/was refused:");
- log_error!(logger, " {}", e);
- ret = Err(());
- }
+ self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone());
  }
  ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => {
  log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -3286,12 +3282,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if ret.is_ok() && (self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed) && is_pre_close_update {
+ if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
  log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
  Err(())
  } else { ret }
 	}
 
+	fn no_further_updates_allowed(&self) -> bool {
+ self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
+	}
+
 	fn get_latest_update_id(&self) -> u64 {
  self.latest_update_id
 	}
@@ -3564,11 +3564,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  return (claimable_outpoints, to_counterparty_output_info);
  }
  let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
+ let counterparty_spendable_height = if htlc.offered {
+ htlc.cltv_expiry
+ } else {
+ height
+ };
  let justice_package = PackageTemplate::build_package(
  commitment_txid,
  transaction_output_index,
  PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
-htlc.cltv_expiry,
+counterparty_spendable_height,
  );
  claimable_outpoints.push(justice_package);
  }
@@ -3869,35 +3874,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
  }
- if self.holder_tx_signed {
- // If we've signed, we may have broadcast either commitment (prev or current), and
- // attempted to claim from it immediately without waiting for a confirmation.
- if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ // Cancel any pending claims for any holder commitments in case they had previously
+ // confirmed or been signed (in which case we will start attempting to claim without
+ // waiting for confirmation).
+ if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+ self.current_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+ if let Some(vout) = htlc.transaction_output_index {
+ outpoint.vout = vout;
+ self.onchain_tx_handler.abandon_claim(&outpoint);
+ }
+ }
+ }
+ if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
+ if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
  log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
-self.current_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+prev_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
  if let Some(vout) = htlc.transaction_output_index {
  outpoint.vout = vout;
  self.onchain_tx_handler.abandon_claim(&outpoint);
  }
  }
  }
- if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
- if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
- log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
- prev_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
- if let Some(vout) = htlc.transaction_output_index {
- outpoint.vout = vout;
- self.onchain_tx_handler.abandon_claim(&outpoint);
- }
- }
- }
- }
- } else {
- // No previous claim.
  }
 	}
 
@@ -4233,7 +4235,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if self.lockdown_from_offchain || self.funding_spend_seen || self.holder_tx_signed {
+ if self.no_further_updates_allowed() {
  // Fail back HTLCs on backwards channels if they expire within
  // `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a
  // point where no further off-chain updates will be accepted). If we haven't seen the
@@ -5384,7 +5386,7 @@ mod tests {
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
 
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()),
  preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()),
@@ -5422,7 +5424,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..5]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
  monitor.provide_secret(281474976710653, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12);
@@ -5433,7 +5435,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..3]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx,
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
  monitor.provide_secret(281474976710652, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5);

lightning/src/chain/package.rs

@@ -699,8 +699,13 @@ impl PackageSolvingData {
  match self {
  PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
- PackageSolvingData::RevokedHTLCOutput(..) =>
- PackageMalleability::Malleable(AggregationCluster::Pinnable),
+ PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
+ if htlc.offered {
+ PackageMalleability::Malleable(AggregationCluster::Unpinnable)
+ } else {
+ PackageMalleability::Malleable(AggregationCluster::Pinnable)
+ }
+ },
  PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
  PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
@@ -771,10 +776,12 @@ pub struct PackageTemplate {
 	/// Block height at which our counterparty can potentially claim this output as well (assuming
 	/// they have the keys or information required to do so).
 	///
-	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
-	/// because the counterparty can potentially spend it. It is also used internally by
-	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
-	/// type of output.
+	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
+	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
+	/// identify when an output must be claimed by, depending on the type of output.
+	///
+	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
+	/// we upgraded from LDK 0.1 or prior.
 	counterparty_spendable_height: u32,
 	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
 	// (either claimed output value or external utxo), it will keep increasing until holder
@@ -834,17 +841,17 @@ impl PackageTemplate {
  // Now check that we only merge packages if they are both unpinnable or both
  // pinnable.
  let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
- self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
- other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_pinnable && other_pinnable {
  return true;
  }
 
  let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
- self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
- other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_unpinnable && other_unpinnable {
  return true;
  }
@@ -855,13 +862,6 @@ impl PackageTemplate {
 	pub(crate) fn is_malleable(&self) -> bool {
  matches!(self.malleability, PackageMalleability::Malleable(..))
 	}
-	/// The height at which our counterparty may be able to spend this output.
-	///
-	/// This is an important limit for aggregation as after this height our counterparty may be
-	/// able to pin transactions spending this output in the mempool.
-	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
- self.counterparty_spendable_height
-	}
 	pub(crate) fn previous_feerate(&self) -> u64 {
  self.feerate_previous
 	}
@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
  (4, _height_original, option), // Written with a dummy value since 0.1
  (6, height_timer, option),
  });
+ for (_, input) in &inputs {
+ if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
+ // LDK versions through 0.1 set the wrong counterparty_spendable_height for
+ // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
+ // claim with a preimage immediately). Here we detect this and reset the value to
+ // zero, as the value is unused except for merging decisions which doesn't care
+ // about any values below the current height.
+ if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
+ counterparty_spendable_height = 0;
+ }
+ }
+ }
  Ok(PackageTemplate {
  inputs,
  malleability,

lightning/src/ln/channelmanager.rs

@@ -13364,8 +13364,8 @@ where
  // claim.
  // Note that a `ChannelMonitor` is created with `update_id` 0 and after we
  // provide it with a closure update its `update_id` will be at 1.
- if !monitor.offchain_closed() || monitor.get_latest_update_id() > 1 {
- should_queue_fc_update = !monitor.offchain_closed();
+ if !monitor.no_further_updates_allowed() || monitor.get_latest_update_id() > 1 {
+ should_queue_fc_update = !monitor.no_further_updates_allowed();
  let mut latest_update_id = monitor.get_latest_update_id();
  if should_queue_fc_update {
  latest_update_id += 1;

.github/workflows/semver.yml

@@ -0,0 +1,57 @@
+name: SemVer checks
+on:
+ push:
+ branches-ignore:
+ - master
+ pull_request:
+ branches-ignore:
+ - master
+
+jobs:
+ semver-checks:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout source code
+ uses: actions/checkout@v4
+ - name: Install Rust stable toolchain
+ run: |
+ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
+ rustup override set stable
+ - name: Check SemVer with default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: default-features
+ - name: Check SemVer *without* default features
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ feature-group: only-explicit-features
+ - name: Check lightning-background-processor SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-background-processor
+ feature-group: only-explicit-features
+ features: futures
+ - name: Check lightning-block-sync SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ package: lightning-block-sync
+ feature-group: only-explicit-features
+ features: rpc-client,rest-client
+ - name: Check lightning-transaction-sync electrum SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: electrum
+ - name: Check lightning-transaction-sync esplora-blocking SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-blocking
+ - name: Check lightning-transaction-sync esplora-async SemVer
+ uses: obi1kenobi/cargo-semver-checks-action@v2
+ with:
+ manifest-path: lightning-transaction-sync/Cargo.toml
+ feature-group: only-explicit-features
+ features: esplora-async

lightning-background-processor/src/lib.rs

@@ -2379,8 +2379,8 @@ mod tests {
  42,
  53,
  features,
- $nodes[0].node.get_our_node_id(),
- $nodes[1].node.get_our_node_id(),
+ $nodes[0].node.get_our_node_id().into(),
+ $nodes[1].node.get_our_node_id().into(),
  )
  .expect("Failed to update channel from partial announcement");
  let original_graph_description = $nodes[0].network_graph.to_string();

lightning/src/chain/channelmonitor.rs

@@ -1509,8 +1509,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	fn provide_latest_holder_commitment_tx(
  &self, holder_commitment_tx: HolderCommitmentTransaction,
  htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
-	) -> Result<(), ()> {
- self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new()).map_err(|_| ())
+	) {
+ self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new())
 	}
 
 	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
@@ -1737,10 +1737,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
  self.inner.lock().unwrap().get_cur_holder_commitment_number()
 	}
 
-	/// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
-	/// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
-	pub(crate) fn offchain_closed(&self) -> bool {
- self.inner.lock().unwrap().lockdown_from_offchain
+	/// Fetches whether this monitor has marked the channel as closed and will refuse any further
+	/// updates to the commitment transactions.
+	///
+	/// It can be marked closed in a few different ways, including via a
+	/// [`ChannelMonitorUpdateStep::ChannelForceClosed`] or if the channel has been closed
+	/// on-chain.
+	pub(crate) fn no_further_updates_allowed(&self) -> bool {
+ self.inner.lock().unwrap().no_further_updates_allowed()
 	}
 
 	/// Gets the `node_id` of the counterparty for this channel.
@@ -2901,7 +2905,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// is important that any clones of this channel monitor (including remote clones) by kept
 	/// up-to-date as our holder commitment transaction is updated.
 	/// Panics if set_on_holder_tx_csv has never been called.
-	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) -> Result<(), &'static str> {
+	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, mut htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], nondust_htlc_sources: Vec<HTLCSource>) {
  if htlc_outputs.iter().any(|(_, s, _)| s.is_some()) {
  // If we have non-dust HTLCs in htlc_outputs, ensure they match the HTLCs in the
  // `holder_commitment_tx`. In the future, we'll no longer provide the redundant data
@@ -2978,10 +2982,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  self.counterparty_fulfilled_htlcs.insert(*claimed_htlc_id, *claimed_preimage);
  }
- if self.holder_tx_signed {
- return Err("Latest holder commitment signed has already been signed, update is rejected");
- }
- Ok(())
 	}
 
 	/// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
@@ -3202,11 +3202,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs, nondust_htlc_sources } => {
  log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
  if self.lockdown_from_offchain { panic!(); }
- if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone()) {
- log_error!(logger, "Providing latest holder commitment transaction failed/was refused:");
- log_error!(logger, " {}", e);
- ret = Err(());
- }
+ self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone());
  }
  ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => {
  log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -3286,12 +3282,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if ret.is_ok() && (self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed) && is_pre_close_update {
+ if ret.is_ok() && self.no_further_updates_allowed() && is_pre_close_update {
  log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
  Err(())
  } else { ret }
 	}
 
+	fn no_further_updates_allowed(&self) -> bool {
+ self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
+	}
+
 	fn get_latest_update_id(&self) -> u64 {
  self.latest_update_id
 	}
@@ -3564,11 +3564,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  return (claimable_outpoints, to_counterparty_output_info);
  }
  let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
+ let counterparty_spendable_height = if htlc.offered {
+ htlc.cltv_expiry
+ } else {
+ height
+ };
  let justice_package = PackageTemplate::build_package(
  commitment_txid,
  transaction_output_index,
  PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
-htlc.cltv_expiry,
+counterparty_spendable_height,
  );
  claimable_outpoints.push(justice_package);
  }
@@ -3869,35 +3874,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
  }
- if self.holder_tx_signed {
- // If we've signed, we may have broadcast either commitment (prev or current), and
- // attempted to claim from it immediately without waiting for a confirmation.
- if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ // Cancel any pending claims for any holder commitments in case they had previously
+ // confirmed or been signed (in which case we will start attempting to claim without
+ // waiting for confirmation).
+ if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
+ log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
+ self.current_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+ if let Some(vout) = htlc.transaction_output_index {
+ outpoint.vout = vout;
+ self.onchain_tx_handler.abandon_claim(&outpoint);
+ }
+ }
+ }
+ if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
+ if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
  log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
-self.current_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
+prev_holder_commitment_tx.txid);
+ let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
+ for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
  if let Some(vout) = htlc.transaction_output_index {
  outpoint.vout = vout;
  self.onchain_tx_handler.abandon_claim(&outpoint);
  }
  }
  }
- if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
- if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
- log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
- prev_holder_commitment_tx.txid);
- let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
- for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
- if let Some(vout) = htlc.transaction_output_index {
- outpoint.vout = vout;
- self.onchain_tx_handler.abandon_claim(&outpoint);
- }
- }
- }
- }
- } else {
- // No previous claim.
  }
 	}
 
@@ -4233,7 +4235,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
  }
  }
 
- if self.lockdown_from_offchain || self.funding_spend_seen || self.holder_tx_signed {
+ if self.no_further_updates_allowed() {
  // Fail back HTLCs on backwards channels if they expire within
  // `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a
  // point where no further off-chain updates will be accepted). If we haven't seen the
@@ -5384,7 +5386,7 @@ mod tests {
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
 
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()),
  preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
  monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()),
@@ -5422,7 +5424,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..5]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
  monitor.provide_secret(281474976710653, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12);
@@ -5433,7 +5435,7 @@ mod tests {
  let mut htlcs = preimages_slice_to_htlcs!(preimages[0..3]);
  let dummy_commitment_tx = HolderCommitmentTransaction::dummy(&mut htlcs);
  monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx,
- htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect()).unwrap();
+ htlcs.into_iter().map(|(htlc, _)| (htlc, Some(dummy_sig), None)).collect());
  secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
  monitor.provide_secret(281474976710652, secret.clone()).unwrap();
  assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5);

lightning/src/chain/package.rs

@@ -699,8 +699,13 @@ impl PackageSolvingData {
  match self {
  PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
- PackageSolvingData::RevokedHTLCOutput(..) =>
- PackageMalleability::Malleable(AggregationCluster::Pinnable),
+ PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
+ if htlc.offered {
+ PackageMalleability::Malleable(AggregationCluster::Unpinnable)
+ } else {
+ PackageMalleability::Malleable(AggregationCluster::Pinnable)
+ }
+ },
  PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
  PackageMalleability::Malleable(AggregationCluster::Unpinnable),
  PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
@@ -771,10 +776,12 @@ pub struct PackageTemplate {
 	/// Block height at which our counterparty can potentially claim this output as well (assuming
 	/// they have the keys or information required to do so).
 	///
-	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
-	/// because the counterparty can potentially spend it. It is also used internally by
-	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
-	/// type of output.
+	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
+	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
+	/// identify when an output must be claimed by, depending on the type of output.
+	///
+	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
+	/// we upgraded from LDK 0.1 or prior.
 	counterparty_spendable_height: u32,
 	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
 	// (either claimed output value or external utxo), it will keep increasing until holder
@@ -834,17 +841,17 @@ impl PackageTemplate {
  // Now check that we only merge packages if they are both unpinnable or both
  // pinnable.
  let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
- self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
- other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_pinnable && other_pinnable {
  return true;
  }
 
  let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
- self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
- other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
+ other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
  if self_unpinnable && other_unpinnable {
  return true;
  }
@@ -855,13 +862,6 @@ impl PackageTemplate {
 	pub(crate) fn is_malleable(&self) -> bool {
  matches!(self.malleability, PackageMalleability::Malleable(..))
 	}
-	/// The height at which our counterparty may be able to spend this output.
-	///
-	/// This is an important limit for aggregation as after this height our counterparty may be
-	/// able to pin transactions spending this output in the mempool.
-	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
- self.counterparty_spendable_height
-	}
 	pub(crate) fn previous_feerate(&self) -> u64 {
  self.feerate_previous
 	}
@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
  (4, _height_original, option), // Written with a dummy value since 0.1
  (6, height_timer, option),
  });
+ for (_, input) in &inputs {
+ if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
+ // LDK versions through 0.1 set the wrong counterparty_spendable_height for
+ // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
+ // claim with a preimage immediately). Here we detect this and reset the value to
+ // zero, as the value is unused except for merging decisions which doesn't care
+ // about any values below the current height.
+ if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
+ counterparty_spendable_height = 0;
+ }
+ }
+ }
  Ok(PackageTemplate {
  inputs,
  malleability,

lightning/src/ln/channelmanager.rs

@@ -13364,8 +13364,8 @@ where
  // claim.
  // Note that a `ChannelMonitor` is created with `update_id` 0 and after we
  // provide it with a closure update its `update_id` will be at 1.
- if !monitor.offchain_closed() || monitor.get_latest_update_id() > 1 {
- should_queue_fc_update = !monitor.offchain_closed();
+ if !monitor.no_further_updates_allowed() || monitor.get_latest_update_id() > 1 {
+ should_queue_fc_update = !monitor.no_further_updates_allowed();
  let mut latest_update_id = monitor.get_latest_update_id();
  if should_queue_fc_update {
  latest_update_id += 1;