Remove the async_signing cfg flag

Now that the core features required for `async_signing` are in place, we can go ahead and expose it publicly (rather than behind a a `cfg`-flag). We still don't have full async support for `get_per_commitment_point`, but only one case in channel reconnection remains. The overall logic may still have some hiccups, but its been in use in production at a major LDK user for some time now. Thus, it doesn't really make sense to hide behind a `cfg`-flag, even if the feature is only 99% complete. Further, the new paths exposed are very restricted to signing operations that run async, so the risk for existing users should be incredibly low.
TheBlueMatt · Dec 16, 2024 · 47ca19d · 47ca19d
1 parent d1e94bd
commit 47ca19d
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 65 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -61,7 +61,6 @@ check-cfg = [
     "cfg(ldk_bench)",
     "cfg(ldk_test_vectors)",
     "cfg(taproot)",
-    "cfg(async_signing)",
     "cfg(require_route_graph_test)",
     "cfg(splicing)",
     "cfg(async_payments)",

diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh
@@ -162,8 +162,6 @@ fi
 echo -e "\n\nTest cfg-flag builds"
 RUSTFLAGS="--cfg=taproot" cargo test --verbose --color always -p lightning
 [ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
-RUSTFLAGS="--cfg=async_signing" cargo test --verbose --color always -p lightning
-[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
 RUSTFLAGS="--cfg=splicing" cargo test --verbose --color always -p lightning
 [ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
 RUSTFLAGS="--cfg=async_payments" cargo test --verbose --color always -p lightning
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -906,7 +906,6 @@ pub(super) struct MonitorRestoreUpdates {
 }
 
 /// The return value of `signer_maybe_unblocked`
-#[allow(unused)]
 pub(super) struct SignerResumeUpdates {
 	pub commitment_update: Option<msgs::CommitmentUpdate>,
 	pub revoke_and_ack: Option<msgs::RevokeAndACK>,
@@ -3959,13 +3958,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			log_trace!(logger, "Counterparty commitment signature available for funding_signed message; clearing signer_pending_funding");
 			self.signer_pending_funding = false;
 		} else if signature.is_none() {
-			#[cfg(not(async_signing))] {
-				panic!("Failed to get signature for funding_signed");
-			}
-			#[cfg(async_signing)] {
-				log_trace!(logger, "Counterparty commitment signature not available for funding_signed message; setting signer_pending_funding");
-				self.signer_pending_funding = true;
-			}
+			log_trace!(logger, "Counterparty commitment signature not available for funding_signed message; setting signer_pending_funding");
+			self.signer_pending_funding = true;
 		}
 
 		signature.map(|(signature, _)| msgs::FundingSigned {
@@ -6114,7 +6108,6 @@ impl<SP: Deref> Channel<SP> where
 
 	/// Indicates that the signer may have some signatures for us, so we should retry if we're
 	/// blocked.
-	#[cfg(async_signing)]
 	pub fn signer_maybe_unblocked<L: Deref>(&mut self, logger: &L) -> SignerResumeUpdates where L::Target: Logger {
 		if !self.holder_commitment_point.is_available() {
 			log_trace!(logger, "Attempting to update holder per-commitment point...");
@@ -6231,21 +6224,16 @@ impl<SP: Deref> Channel<SP> where
 				&self.context.channel_id(), self.holder_commitment_point.transaction_number(),
 				self.holder_commitment_point.transaction_number() + 2);
 		}
-		#[cfg(not(async_signing))] {
-			panic!("Holder commitment point and per commitment secret must be available when generating revoke_and_ack");
-		}
-		#[cfg(async_signing)] {
-			// Technically if we're at HolderCommitmentPoint::PendingNext,
-			// we have a commitment point ready to send in an RAA, however we
-			// choose to wait since if we send RAA now, we could get another
-			// CS before we have any commitment point available. Blocking our
-			// RAA here is a convenient way to make sure that post-funding
-			// we're only ever waiting on one commitment point at a time.
-			log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
-				&self.context.channel_id(), self.holder_commitment_point.transaction_number());
-			self.context.signer_pending_revoke_and_ack = true;
-			None
-		}
+		// Technically if we're at HolderCommitmentPoint::PendingNext,
+		// we have a commitment point ready to send in an RAA, however we
+		// choose to wait since if we send RAA now, we could get another
+		// CS before we have any commitment point available. Blocking our
+		// RAA here is a convenient way to make sure that post-funding
+		// we're only ever waiting on one commitment point at a time.
+		log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
+			&self.context.channel_id(), self.holder_commitment_point.transaction_number());
+		self.context.signer_pending_revoke_and_ack = true;
+		None
 	}
 
 	/// Gets the last commitment update for immediate sending to our peer.
@@ -6316,16 +6304,11 @@ impl<SP: Deref> Channel<SP> where
 			}
 			update
 		} else {
-			#[cfg(not(async_signing))] {
-				panic!("Failed to get signature for new commitment state");
-			}
-			#[cfg(async_signing)] {
-				if !self.context.signer_pending_commitment_update {
-					log_trace!(logger, "Commitment update awaiting signer: setting signer_pending_commitment_update");
-					self.context.signer_pending_commitment_update = true;
-				}
-				return Err(());
+			if !self.context.signer_pending_commitment_update {
+				log_trace!(logger, "Commitment update awaiting signer: setting signer_pending_commitment_update");
+				self.context.signer_pending_commitment_update = true;
 			}
+			return Err(());
 		};
 		Ok(msgs::CommitmentUpdate {
 			update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs, update_fee,
@@ -8362,13 +8345,8 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 			log_trace!(logger, "Counterparty commitment signature ready for funding_created message: clearing signer_pending_funding");
 			self.context.signer_pending_funding = false;
 		} else if signature.is_none() {
-			#[cfg(not(async_signing))] {
-				panic!("Failed to get signature for new funding creation");
-			}
-			#[cfg(async_signing)] {
-				log_trace!(logger, "funding_created awaiting signer; setting signer_pending_funding");
-				self.context.signer_pending_funding = true;
-			}
+			log_trace!(logger, "funding_created awaiting signer; setting signer_pending_funding");
+			self.context.signer_pending_funding = true;
 		};
 
 		signature.map(|signature| msgs::FundingCreated {
@@ -8467,14 +8445,9 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 				holder_commitment_point.current_point()
 			},
 			_ => {
-				#[cfg(not(async_signing))] {
-					panic!("Failed getting commitment point for open_channel message");
-				}
-				#[cfg(async_signing)] {
-					log_trace!(_logger, "Unable to generate open_channel message, waiting for commitment point");
-					self.signer_pending_open_channel = true;
-					return None;
-				}
+				log_trace!(_logger, "Unable to generate open_channel message, waiting for commitment point");
+				self.signer_pending_open_channel = true;
+				return None;
 			}
 		};
 		let keys = self.context.get_holder_pubkeys();
@@ -8562,7 +8535,6 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 
 	/// Indicates that the signer may have some signatures for us, so we should retry if we're
 	/// blocked.
-	#[cfg(async_signing)]
 	pub fn signer_maybe_unblocked<L: Deref>(
 		&mut self, chain_hash: ChainHash, logger: &L
 	) -> (Option<msgs::OpenChannel>, Option<msgs::FundingCreated>) where L::Target: Logger {
@@ -8723,14 +8695,9 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 				holder_commitment_point.current_point()
 			},
 			_ => {
-				#[cfg(not(async_signing))] {
-					panic!("Failed getting commitment point for accept_channel message");
-				}
-				#[cfg(async_signing)] {
-					log_trace!(_logger, "Unable to generate accept_channel message, waiting for commitment point");
-					self.signer_pending_accept_channel = true;
-					return None;
-				}
+				log_trace!(_logger, "Unable to generate accept_channel message, waiting for commitment point");
+				self.signer_pending_accept_channel = true;
+				return None;
 			}
 		};
 		let keys = self.context.get_holder_pubkeys();
@@ -8833,7 +8800,6 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 
 	/// Indicates that the signer may have some signatures for us, so we should retry if we're
 	/// blocked.
-	#[allow(unused)]
 	pub fn signer_maybe_unblocked<L: Deref>(
 		&mut self, logger: &L
 	) -> Option<msgs::AcceptChannel> where L::Target: Logger {

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -9512,7 +9512,6 @@ where
 	/// attempted in every channel, or in the specifically provided channel.
 	///
 	/// [`ChannelSigner`]: crate::sign::ChannelSigner
-	#[cfg(async_signing)]
 	pub fn signer_unblocked(&self, channel_opt: Option<(PublicKey, ChannelId)>) {
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 

diff --git a/lightning/src/ln/mod.rs b/lightning/src/ln/mod.rs
@@ -85,7 +85,7 @@ mod monitor_tests;
 #[cfg(test)]
 #[allow(unused_mut)]
 mod shutdown_tests;
-#[cfg(all(test, async_signing))]
+#[cfg(test)]
 #[allow(unused_mut)]
 mod async_signer_tests;
 #[cfg(test)]

diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs
@@ -728,8 +728,7 @@ pub trait ChannelSigner {
 	/// Note that the commitment number starts at `(1 << 48) - 1` and counts backwards.
 	///
 	/// If the signer returns `Err`, then the user is responsible for either force-closing the channel
-	/// or calling `ChannelManager::signer_unblocked` (this method is only available when the
-	/// `async_signing` cfg flag is enabled) once the signature is ready.
+	/// or calling `ChannelManager::signer_unblocked` once the signature is ready.
 	///
 	// TODO: link to `signer_unblocked` once the cfg flag is removed
 	fn get_per_commitment_point(