Merge PR 'Move holder commit sig checks to `InMemorySigner`' (#4885)

### fuzz/src/chanmon_consistency.rs
@@ -764,6 +764,7 @@ struct KeyProvider {
 	node_secret: SecretKey,
 	rand_bytes_id: atomic::AtomicU32,
 	enforcement_states: Mutex<HashMap<[u8; 32], Arc<Mutex<EnforcementState>>>>,
+	logger: Arc<dyn Logger + MaybeSend + MaybeSync>,
 }
 
 impl EntropySource for KeyProvider {
@@ -862,6 +863,7 @@ impl SignerProvider for KeyProvider {
 			[id, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, self.node_secret[31]],
 			channel_keys_id,
 			channel_keys_id,
+			Arc::clone(&self.logger),
 		);
 		let revoked_commitment = self.make_enforcement_state_cell(keys.commitment_seed);
 		let keys = DynSigner::new(keys);
@@ -1162,6 +1164,7 @@ impl<'a> HarnessNode<'a> {
 			node_secret,
 			rand_bytes_id: atomic::AtomicU32::new(0),
 			enforcement_states: Mutex::new(new_hash_map()),
+			logger: Arc::clone(&logger),
 		});
 		let persister = Self::build_persister(persistence_style);
 		let monitor = Self::build_chain_monitor(

### fuzz/src/full_stack.rs
@@ -388,6 +388,7 @@ struct KeyProvider {
 	counter: AtomicU64,
 	signer_state: RefCell<HashMap<u8, (bool, Arc<Mutex<EnforcementState>>)>>,
 	rng_output: RefCell<[u8; 32]>,
+	logger: Arc<dyn Logger + MaybeSend + MaybeSync>,
 }
 
 impl EntropySource for KeyProvider {
@@ -487,7 +488,19 @@ impl SignerProvider for KeyProvider {
 		f = key;
 		// We leave both the v1 and v2 derivation to_remote keys the same as there's not any real
 		// reason to fuzz differences here, and it keeps us consistent with past behavior.
-		let signer = InMemorySigner::new(a, b, c, c, true, d, e, f, keys_id, keys_id);
+		let signer = InMemorySigner::new(
+			a,
+			b,
+			c,
+			c,
+			true,
+			d,
+			e,
+			f,
+			keys_id,
+			keys_id,
+			Arc::clone(&self.logger),
+		);
 
 		TestChannelSigner::new_with_revoked(DynSigner::new(signer), state, false, false)
 	}
@@ -591,6 +604,7 @@ pub fn do_test(mut data: &[u8], logger: &Arc<dyn Logger + MaybeSend + MaybeSync>
 		counter: AtomicU64::new(0),
 		signer_state: RefCell::new(new_hash_map()),
 		rng_output: RefCell::new([42; 32]),
+		logger: Arc::clone(logger),
 	});
 
 	let monitor = Arc::new(chainmonitor::ChainMonitor::new(

### fuzz/src/lsps_message.rs
@@ -38,7 +38,13 @@ pub fn do_test(data: &[u8]) {
 	let scorer = Arc::new(LockingWrapper::new(TestScorer::new()));
 	let now = Duration::from_secs(genesis_block.header.time as u64);
 	let seed = sha256::Hash::hash(b"lsps-message-seed").to_byte_array();
-	let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos(), true));
+	let keys_manager = Arc::new(KeysManager::new(
+		&seed,
+		now.as_secs(),
+		now.subsec_nanos(),
+		true,
+		Arc::clone(&logger),
+	));
 	let router = Arc::new(DefaultRouter::new(
 		Arc::clone(&network_graph),
 		Arc::clone(&logger),

### lightning-background-processor/src/lib.rs
@@ -379,13 +379,16 @@ type DynMessageRouter = lightning::onion_message::messenger::DefaultMessageRoute
 >;
 
 #[cfg(not(c_bindings))]
-type DynSignerProvider = dyn lightning::sign::SignerProvider<EcdsaSigner = lightning::sign::InMemorySigner>
-	+ Send
+type DynSignerProvider = dyn lightning::sign::SignerProvider<
+		EcdsaSigner = lightning::sign::InMemorySigner<&'static (dyn Logger + Send + Sync)>,
+	> + Send
 	+ Sync;
 
 #[cfg(not(c_bindings))]
 type DynChannelManager = lightning::ln::channelmanager::ChannelManager<
-	&'static (dyn chain::Watch<lightning::sign::InMemorySigner> + Send + Sync),
+	&'static (dyn chain::Watch<lightning::sign::InMemorySigner<&'static (dyn Logger + Send + Sync)>>
+	              + Send
+	              + Sync),
 	&'static (dyn BroadcasterInterface + Send + Sync),
 	&'static (dyn EntropySource + Send + Sync),
 	&'static (dyn lightning::sign::NodeSigner + Send + Sync),
@@ -826,12 +829,12 @@ use futures_util::{dummy_waker, Joiner, OptionalSelector, Selector, SelectorOutp
 /// #     fn send_data(&mut self, _data: &[u8], _continue_read: bool) -> usize { 0 }
 /// #     fn disconnect_socket(&mut self) {}
 /// # }
-/// # type ChainMonitor<B, F, FE> = lightning::chain::chainmonitor::ChainMonitor<lightning::sign::InMemorySigner, Arc<F>, Arc<B>, Arc<FE>, Arc<Logger>, Arc<StoreSync>, Arc<lightning::sign::KeysManager>>;
+/// # type ChainMonitor<B, F, FE> = lightning::chain::chainmonitor::ChainMonitor<lightning::sign::InMemorySigner<Arc<Logger>>, Arc<F>, Arc<B>, Arc<FE>, Arc<Logger>, Arc<StoreSync>, Arc<lightning::sign::KeysManager<Arc<Logger>>>>;
 /// # type NetworkGraph = lightning::routing::gossip::NetworkGraph<Arc<Logger>>;
 /// # type P2PGossipSync<UL> = lightning::routing::gossip::P2PGossipSync<Arc<NetworkGraph>, Arc<UL>, Arc<Logger>>;
 /// # type ChannelManager<B, F, FE> = lightning::ln::channelmanager::SimpleArcChannelManager<ChainMonitor<B, F, FE>, B, FE, Logger>;
-/// # type OnionMessenger<B, F, FE> = lightning::onion_message::messenger::OnionMessenger<Arc<lightning::sign::KeysManager>, Arc<lightning::sign::KeysManager>, Arc<Logger>, Arc<ChannelManager<B, F, FE>>, Arc<lightning::onion_message::messenger::DefaultMessageRouter<Arc<NetworkGraph>, Arc<Logger>, Arc<lightning::sign::KeysManager>>>, Arc<ChannelManager<B, F, FE>>, lightning::ln::peer_handler::IgnoringMessageHandler, lightning::ln::peer_handler::IgnoringMessageHandler, lightning::ln::peer_handler::IgnoringMessageHandler>;
-/// # type LiquidityManager<B, F, FE> = lightning_liquidity::LiquidityManager<Arc<lightning::sign::KeysManager>, Arc<lightning::sign::KeysManager>, Arc<ChannelManager<B, F, FE>>, Arc<Store>, Arc<DefaultTimeProvider>, Arc<B>>;
+/// # type OnionMessenger<B, F, FE> = lightning::onion_message::messenger::OnionMessenger<Arc<lightning::sign::KeysManager<Arc<Logger>>>, Arc<lightning::sign::KeysManager<Arc<Logger>>>, Arc<Logger>, Arc<ChannelManager<B, F, FE>>, Arc<lightning::onion_message::messenger::DefaultMessageRouter<Arc<NetworkGraph>, Arc<Logger>, Arc<lightning::sign::KeysManager<Arc<Logger>>>>>, Arc<ChannelManager<B, F, FE>>, lightning::ln::peer_handler::IgnoringMessageHandler, lightning::ln::peer_handler::IgnoringMessageHandler, lightning::ln::peer_handler::IgnoringMessageHandler>;
+/// # type LiquidityManager<B, F, FE> = lightning_liquidity::LiquidityManager<Arc<lightning::sign::KeysManager<Arc<Logger>>>, Arc<lightning::sign::KeysManager<Arc<Logger>>>, Arc<ChannelManager<B, F, FE>>, Arc<Store>, Arc<DefaultTimeProvider>, Arc<B>>;
 /// # type Scorer = RwLock<lightning::routing::scoring::ProbabilisticScorer<Arc<NetworkGraph>, Arc<Logger>>>;
 /// # type PeerManager<B, F, FE, UL> = lightning::ln::peer_handler::SimpleArcPeerManager<SocketDescriptor, ChainMonitor<B, F, FE>, B, FE, Arc<UL>, Logger, F, StoreSync>;
 /// # type OutputSweeper<B, D, FE, F, O> = lightning::util::sweep::OutputSweeper<Arc<B>, Arc<D>, Arc<FE>, Arc<F>, Arc<Store>, Arc<Logger>, Arc<O>>;
@@ -1962,7 +1965,7 @@ mod tests {
 	use lightning::routing::gossip::{NetworkGraph, P2PGossipSync};
 	use lightning::routing::router::{CandidateRouteHop, DefaultRouter, Path, RouteHop};
 	use lightning::routing::scoring::{ChannelUsage, LockableScore, ScoreLookUp, ScoreUpdate};
-	use lightning::sign::{ChangeDestinationSourceSync, InMemorySigner, KeysManager, NodeSigner};
+	use lightning::sign::{ChangeDestinationSourceSync, NodeSigner};
 	use lightning::types::features::{ChannelFeatures, NodeFeatures};
 	use lightning::types::payment::PaymentHash;
 	use lightning::util::config::UserConfig;
@@ -1993,6 +1996,8 @@ mod tests {
 
 	const EVENT_DEADLINE: Duration =
 		Duration::from_millis(5 * (FRESHNESS_TIMER.as_millis() as u64));
+	type InMemorySigner = lightning::sign::InMemorySigner<Arc<test_utils::TestLogger>>;
+	type KeysManager = lightning::sign::KeysManager<Arc<test_utils::TestLogger>>;
 
 	/// Reads a directory and returns only non-`.tmp` files.
 	/// The file system may return files in any order, and during persistence
@@ -2464,8 +2469,13 @@ mod tests {
 			let scorer = Arc::new(LockingWrapper::new(TestScorer::new()));
 			let now = Duration::from_secs(genesis_block.header.time as u64);
 			let seed = [i as u8; 32];
-			let keys_manager =
-				Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos(), true));
+			let keys_manager = Arc::new(KeysManager::new(
+				&seed,
+				now.as_secs(),
+				now.subsec_nanos(),
+				true,
+				Arc::clone(&logger),
+			));
 			let router = Arc::new(DefaultRouter::new(
 				Arc::clone(&network_graph),
 				Arc::clone(&logger),
@@ -2481,8 +2491,13 @@ mod tests {
 			let kv_store =
 				Arc::new(Persister::new(format!("{}_persister_{}", &persist_dir, i).into()));
 			let now = Duration::from_secs(genesis_block.header.time as u64);
-			let keys_manager =
-				Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos(), true));
+			let keys_manager = Arc::new(KeysManager::new(
+				&seed,
+				now.as_secs(),
+				now.subsec_nanos(),
+				true,
+				Arc::clone(&logger),
+			));
 			let chain_monitor = Arc::new(chainmonitor::ChainMonitor::new(
 				Some(Arc::clone(&chain_source)),
 				Arc::clone(&tx_broadcaster),

### lightning-dns-resolver/src/lib.rs
@@ -185,6 +185,7 @@ mod test {
 	use std::sync::Mutex;
 	use std::time::{Duration, Instant, SystemTime};
 
+	#[derive(Clone, Copy)]
 	struct TestLogger {
 		node: &'static str,
 	}
@@ -217,7 +218,8 @@ mod test {
 			&self, recipient: PublicKey, local_node_receive_key: ReceiveAuthKey,
 			context: MessageContext, _peers: Vec<MessageForwardNode>, secp_ctx: &Secp256k1<T>,
 		) -> Result<Vec<BlindedMessagePath>, ()> {
-			let keys = KeysManager::new(&[0; 32], 42, 43, true);
+			let logger = TestLogger { node: "router" };
+			let keys = KeysManager::new(&[0; 32], 42, 43, true, logger);
 			Ok(vec![BlindedMessagePath::one_hop(
 				recipient,
 				local_node_receive_key,
@@ -265,8 +267,8 @@ mod test {
 	}
 
 	fn create_resolver() -> (impl AOnionMessenger, PublicKey) {
-		let resolver_keys = Arc::new(KeysManager::new(&[99; 32], 42, 43, true));
 		let resolver_logger = TestLogger { node: "resolver" };
+		let resolver_keys = Arc::new(KeysManager::new(&[99; 32], 42, 43, true, resolver_logger));
 		let resolver = OMDomainResolver::ignoring_incoming_proofs("8.8.8.8:53".parse().unwrap());
 		let resolver = Arc::new(resolver);
 		(
@@ -302,8 +304,8 @@ mod test {
 		let payment_id = PaymentId([42; 32]);
 		let name = HumanReadableName::from_encoded("matt@mattcorallo.com").unwrap();
 
-		let payer_keys = Arc::new(KeysManager::new(&[2; 32], 42, 43, true));
 		let payer_logger = TestLogger { node: "payer" };
+		let payer_keys = Arc::new(KeysManager::new(&[2; 32], 42, 43, true, payer_logger));
 		let payer_id = payer_keys.get_node_id(Recipient::Node).unwrap();
 		let payer = Arc::new(URIResolver {
 			resolved_uri: Mutex::new(None),
@@ -369,8 +371,8 @@ mod test {
 		let name =
 			HumanReadableName::from_encoded("nonexistent-user-ldk-test@mattcorallo.com").unwrap();
 
-		let payer_keys = Arc::new(KeysManager::new(&[3; 32], 42, 43, true));
 		let payer_logger = TestLogger { node: "payer" };
+		let payer_keys = Arc::new(KeysManager::new(&[3; 32], 42, 43, true, payer_logger));
 		let payer_id = payer_keys.get_node_id(Recipient::Node).unwrap();
 		let payer = Arc::new(URIResolver {
 			resolved_uri: Mutex::new(None),
@@ -428,8 +430,8 @@ mod test {
 
 		// Resolver points at a port that should refuse TCP, so build_txt_proof_async
 		// returns Err quickly.
-		let resolver_keys = Arc::new(KeysManager::new(&[99; 32], 42, 43, true));
 		let resolver_logger = TestLogger { node: "resolver" };
+		let resolver_keys = Arc::new(KeysManager::new(&[99; 32], 42, 43, true, resolver_logger));
 		let resolver =
 			Arc::new(OMDomainResolver::<IgnoringMessageHandler>::ignoring_incoming_proofs(
 				"127.0.0.1:1".parse().unwrap(),
@@ -454,8 +456,8 @@ mod test {
 		let payment_id = PaymentId([42; 32]);
 		let name = HumanReadableName::from_encoded("matt@mattcorallo.com").unwrap();
 
-		let payer_keys = Arc::new(KeysManager::new(&[2; 32], 42, 43, true));
 		let payer_logger = TestLogger { node: "payer" };
+		let payer_keys = Arc::new(KeysManager::new(&[2; 32], 42, 43, true, payer_logger));
 		let payer_id = payer_keys.get_node_id(Recipient::Node).unwrap();
 		let payer = Arc::new(URIResolver {
 			resolved_uri: Mutex::new(None),

### lightning/src/chain/channelmonitor.rs
@@ -7094,7 +7094,10 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 
 #[cfg(test)]
 pub(super) fn dummy_monitor<S: EcdsaChannelSigner + 'static>(
-	channel_id: ChannelId, wrap_signer: impl FnOnce(crate::sign::InMemorySigner) -> S,
+	channel_id: ChannelId,
+	wrap_signer: impl FnOnce(
+		crate::sign::InMemorySigner<crate::sync::Arc<crate::util::test_utils::TestLogger>>,
+	) -> S,
 ) -> ChannelMonitor<S> {
 	use crate::ln::chan_utils::{ChannelPublicKeys, CounterpartyChannelTransactionParameters};
 	use crate::sign::{ChannelSigner, InMemorySigner};
@@ -7114,6 +7117,7 @@ pub(super) fn dummy_monitor<S: EcdsaChannelSigner + 'static>(
 		[41; 32],
 		[0; 32],
 		[0; 32],
+		crate::sync::Arc::new(crate::util::test_utils::TestLogger::new()),
 	);
 	let counterparty_pubkeys = ChannelPublicKeys {
 		funding_pubkey: dummy_key,

### lightning/src/chain/onchaintx.rs
@@ -1322,6 +1322,7 @@ mod tests {
 	#[rustfmt::skip]
 	fn test_broadcast_height() {
 		let secp_ctx = Secp256k1::new();
+		let logger = TestLogger::new();
 		let signer = InMemorySigner::new(
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			SecretKey::from_slice(&[41; 32]).unwrap(),
@@ -1333,6 +1334,7 @@ mod tests {
 			[41; 32],
 			[0; 32],
 			[0; 32],
+			&logger,
 		);
 		let counterparty_pubkeys = ChannelPublicKeys {
 			funding_pubkey: PublicKey::from_secret_key(
@@ -1415,8 +1417,6 @@ mod tests {
 
 		let fee_estimator = TestFeeEstimator::new(253);
 		let fee_estimator = LowerBoundedFeeEstimator::new(&fee_estimator);
-		let logger = TestLogger::new();
-
 		// Request claiming of each HTLC on the holder's commitment, with current block height 1.
 		let holder_commit = tx_handler.current_holder_commitment_tx();
 		let holder_commit_txid = holder_commit.trust().txid();

### lightning/src/events/bump_transaction/mod.rs
@@ -979,8 +979,8 @@ mod tests {
 				),
 			]),
 		};
-		let signer = KeysManager::new(&[42; 32], 42, 42, true);
 		let logger = TestLogger::new();
+		let signer = KeysManager::new(&[42; 32], 42, 42, true, &logger);
 		let handler = BumpTransactionEventHandlerSync::new(&broadcaster, &source, &signer, &logger);
 
 		let mut transaction_parameters = ChannelTransactionParameters::test_dummy(42_000_000);

### lightning/src/ln/channel.rs
@@ -24,7 +24,7 @@ use bitcoin::hashes::Hash;
 use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE;
 use bitcoin::secp256k1::{ecdsa::Signature, Secp256k1};
 use bitcoin::secp256k1::{PublicKey, SecretKey};
-use bitcoin::{secp256k1, sighash, FeeRate, Sequence, TxIn};
+use bitcoin::{secp256k1, FeeRate, Sequence, TxIn};
 
 use crate::blinded_path::message::BlindedMessagePath;
 use crate::chain::chaininterface::{
@@ -3963,69 +3963,64 @@ trait InitialRemoteCommitmentReceiver<SP: SignerProvider> {
 
 	fn received_msg(&self) -> &'static str;
 
-	#[rustfmt::skip]
-	fn check_counterparty_commitment_signature<L: Logger>(
-		&self, sig: &Signature, holder_commitment_point: &HolderCommitmentPoint, logger: &L
-	) -> Result<CommitmentTransaction, ChannelError> {
-		let funding_script = self.funding().get_funding_redeemscript();
-
-		let commitment_data = self.context().build_commitment_transaction(self.funding(),
-			holder_commitment_point.next_transaction_number(), &holder_commitment_point.next_point(),
-			true, false, logger);
-		let initial_commitment_tx = commitment_data.tx;
-		let trusted_tx = initial_commitment_tx.trust();
-		let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
-		let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.funding().get_value_satoshis());
-		// They sign the holder commitment transaction...
-		log_trace!(logger, "Checking {} tx signature {} by key {} against tx {} (sighash {}) with redeemscript {} for channel {}.",
-			self.received_msg(), log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.funding().counterparty_funding_pubkey().serialize()),
-			encode::serialize_hex(&initial_commitment_bitcoin_tx.transaction), log_bytes!(sighash[..]),
-			encode::serialize_hex(&funding_script), &self.context().channel_id());
-		secp_check!(self.context().secp_ctx.verify_ecdsa(&sighash, sig, self.funding().counterparty_funding_pubkey()), format!("Invalid {} signature from peer", self.received_msg()));
-
-		Ok(initial_commitment_tx)
-	}
-
-	#[rustfmt::skip]
 	fn initial_commitment_signed<L: Logger>(
-		&mut self, channel_id: ChannelId, counterparty_signature: Signature, holder_commitment_point: &mut HolderCommitmentPoint,
-		best_block: BlockLocator, signer_provider: &SP, logger: &L,
+		&mut self, channel_id: ChannelId, counterparty_signature: Signature,
+		holder_commitment_point: &mut HolderCommitmentPoint, best_block: BlockLocator,
+		signer_provider: &SP, logger: &L,
 	) -> Result<(ChannelMonitor<SP::EcdsaSigner>, CommitmentTransaction), ChannelError> {
-		let initial_commitment_tx = match self.check_counterparty_commitment_signature(&counterparty_signature, holder_commitment_point, logger) {
-			Ok(res) => res,
-			Err(ChannelError::Close(e)) => {
-				// TODO(dual_funding): Update for V2 established channels.
-				if !self.funding().is_outbound() {
-					self.funding_mut().channel_transaction_parameters.funding_outpoint = None;
-				}
-				return Err(ChannelError::Close(e));
-			},
-			Err(e) => {
-				// The only error we know how to handle is ChannelError::Close, so we fall over here
-				// to make sure we don't continue with an inconsistent state.
-				panic!("unexpected error type from check_counterparty_commitment_signature {:?}", e);
-			}
-		};
 		let context = self.context();
-		let commitment_data = context.build_commitment_transaction(self.funding(),
+
+		let remote_commitment_data = context.build_commitment_transaction(
+			self.funding(),
 			context.counterparty_next_commitment_transaction_number,
-			&context.counterparty_next_commitment_point.unwrap(), false, false, logger);
-		let counterparty_initial_commitment_tx = commitment_data.tx;
+			&context.counterparty_next_commitment_point.unwrap(),
+			false,
+			false,
+			logger,
+		);
+		let counterparty_initial_commitment_tx = remote_commitment_data.tx;
 		let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
 		let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
 
-		log_trace!(logger, "Initial counterparty tx for channel {} is: txid {} tx {}",
-			&context.channel_id(), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
+		log_trace!(
+			logger,
+			"Initial counterparty tx for channel {} is: txid {} tx {}",
+			&context.channel_id(),
+			counterparty_initial_bitcoin_tx.txid,
+			encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction)
+		);
+
+		let local_commitment_data = self.context().build_commitment_transaction(
+			self.funding(),
+			holder_commitment_point.next_transaction_number(),
+			&holder_commitment_point.next_point(),
+			true,
+			false,
+			logger,
+		);
 
 		let holder_commitment_tx = HolderCommitmentTransaction::new(
-			initial_commitment_tx,
+			local_commitment_data.tx,
 			counterparty_signature,
 			Vec::new(),
 			&self.funding().get_holder_pubkeys().funding_pubkey,
-			&self.funding().counterparty_funding_pubkey()
+			&self.funding().counterparty_funding_pubkey(),
 		);
 
-		if context.holder_signer.validate_holder_commitment(&holder_commitment_tx, Vec::new()).is_err() {
+		if context
+			.holder_signer
+			.validate_holder_commitment(
+				&self.funding().channel_transaction_parameters,
+				&holder_commitment_tx,
+				Vec::new(),
+				&context.secp_ctx,
+			)
+			.is_err()
+		{
+			// TODO(dual_funding): Update for V2 established channels.
+			if !self.funding().is_outbound() {
+				self.funding_mut().channel_transaction_parameters.funding_outpoint = None;
+			}
 			return Err(ChannelError::close("Failed to validate our commitment".to_owned()));
 		}
 
@@ -4038,37 +4033,65 @@ trait InitialRemoteCommitmentReceiver<SP: SignerProvider> {
 		assert!(!context.channel_state.is_monitor_update_in_progress()); // We have not had any monitor(s) yet to fail update!
 		if !is_v2_established {
 			if context.is_batch_funding() {
-				context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::WAITING_FOR_BATCH);
+				context.channel_state = ChannelState::AwaitingChannelReady(
+					AwaitingChannelReadyFlags::WAITING_FOR_BATCH,
+				);
 			} else {
-				context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
+				context.channel_state =
+					ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
 			}
 		}
-		if holder_commitment_point.advance(&context.holder_signer, &context.secp_ctx, logger).is_err() {
+		if holder_commitment_point
+			.advance(&context.holder_signer, &context.secp_ctx, logger)
+			.is_err()
+		{
 			// We only fail to advance our commitment point/number if we're currently
 			// waiting for our signer to unblock and provide a commitment point.
 			// We cannot send accept_channel/open_channel before this has occurred, so if we
 			// err here by the time we receive funding_created/funding_signed, something has gone wrong.
-			debug_assert!(false, "We should be ready to advance our commitment point by the time we receive {}", self.received_msg());
-			return Err(ChannelError::close("Failed to advance holder commitment point".to_owned()));
+			debug_assert!(
+				false,
+				"We should be ready to advance our commitment point by the time we receive {}",
+				self.received_msg()
+			);
+			// TODO(dual_funding): Update for V2 established channels.
+			if !self.funding().is_outbound() {
+				self.funding_mut().channel_transaction_parameters.funding_outpoint = None;
+			}
+			return Err(ChannelError::close(
+				"Failed to advance holder commitment point".to_owned(),
+			));
 		}
 
 		let context = self.context();
 		let funding = self.funding();
-		let obscure_factor = get_commitment_transaction_number_obscure_factor(&funding.get_holder_pubkeys().payment_point, &funding.get_counterparty_pubkeys().payment_point, funding.is_outbound());
-		let shutdown_script = context.shutdown_scriptpubkey.clone().map(|script| script.into_inner());
+		let obscure_factor = get_commitment_transaction_number_obscure_factor(
+			&funding.get_holder_pubkeys().payment_point,
+			&funding.get_counterparty_pubkeys().payment_point,
+			funding.is_outbound(),
+		);
+		let shutdown_script =
+			context.shutdown_scriptpubkey.clone().map(|script| script.into_inner());
 		let monitor_signer = signer_provider.derive_channel_signer(context.channel_keys_id);
 		// TODO(RBF): When implementing RBF, the funding_txo passed here must only update
 		// ChannelMonitorImp::first_confirmed_funding_txo during channel establishment, not splicing
 		let channel_monitor = ChannelMonitor::new(
-			context.secp_ctx.clone(), monitor_signer, shutdown_script,
-			funding.get_holder_selected_contest_delay(), &context.destination_script,
-			&funding.channel_transaction_parameters, funding.is_outbound(), obscure_factor,
-			holder_commitment_tx, best_block, context.counterparty_node_id, context.channel_id(),
+			context.secp_ctx.clone(),
+			monitor_signer,
+			shutdown_script,
+			funding.get_holder_selected_contest_delay(),
+			&context.destination_script,
+			&funding.channel_transaction_parameters,
+			funding.is_outbound(),
+			obscure_factor,
+			holder_commitment_tx,
+			best_block,
+			context.counterparty_node_id,
+			context.channel_id(),
 			context.is_manual_broadcast,
 		);
-		channel_monitor.provide_initial_counterparty_commitment_tx(
-			counterparty_initial_commitment_tx.clone(),
-		);
+		channel_monitor
+			.provide_initial_counterparty_commitment_tx(counterparty_initial_commitment_tx.clone());
 
 		self.context_mut().counterparty_next_commitment_transaction_number -= 1;
 
@@ -6047,7 +6070,13 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 		(HolderCommitmentTransaction, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>),
 		ChannelError,
 	> {
-		let funding_script = funding.get_funding_redeemscript();
+		// If our counterparty updated the channel fee in this commitment transaction, check that
+		// they can actually afford the new fee now.
+		if let Some((new_feerate_per_kw, FeeUpdateState::RemoteAnnounced)) = self.pending_update_fee
+		{
+			debug_assert!(!funding.is_outbound());
+			self.validate_update_fee(funding, fee_estimator, new_feerate_per_kw)?;
+		}
 
 		let commitment_data = self.build_commitment_transaction(
 			funding,
@@ -6057,43 +6086,6 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 			false,
 			logger,
 		);
-		let commitment_txid = {
-			let trusted_tx = commitment_data.tx.trust();
-			let bitcoin_tx = trusted_tx.built_transaction();
-			if bitcoin_tx.transaction.output.is_empty() {
-				return Err(ChannelError::close(
-					"Commitment tx from peer has 0 outputs".to_owned(),
-				));
-			}
-
-			let sighash = bitcoin_tx.get_sighash_all(&funding_script, funding.get_value_satoshis());
-
-			log_trace!(logger, "Checking commitment tx signature {} by key {} against tx {} (sighash {}) with redeemscript {} in channel {}",
-				log_bytes!(msg.signature.serialize_compact()[..]),
-				log_bytes!(funding.counterparty_funding_pubkey().serialize()),
-				encode::serialize_hex(&bitcoin_tx.transaction),
-				log_bytes!(sighash[..]), encode::serialize_hex(&funding_script),
-				&self.channel_id(),
-			);
-			if let Err(_) = self.secp_ctx.verify_ecdsa(
-				&sighash,
-				&msg.signature,
-				&funding.counterparty_funding_pubkey(),
-			) {
-				return Err(ChannelError::close(
-					"Invalid commitment tx signature from peer".to_owned(),
-				));
-			}
-			bitcoin_tx.txid
-		};
-
-		// If our counterparty updated the channel fee in this commitment transaction, check that
-		// they can actually afford the new fee now.
-		if let Some((new_feerate_per_kw, FeeUpdateState::RemoteAnnounced)) = self.pending_update_fee
-		{
-			debug_assert!(!funding.is_outbound());
-			self.validate_update_fee(funding, fee_estimator, new_feerate_per_kw)?;
-		}
 
 		if msg.htlc_signatures.len() != commitment_data.tx.nondust_htlcs().len() {
 			return Err(ChannelError::close(format!(
@@ -6103,58 +6095,6 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 			)));
 		}
 
-		let holder_keys = commitment_data.tx.trust().keys();
-		for (htlc, counterparty_sig) in
-			commitment_data.tx.nondust_htlcs().iter().zip(msg.htlc_signatures.iter())
-		{
-			assert!(htlc.transaction_output_index.is_some());
-			let htlc_tx = chan_utils::build_htlc_transaction(
-				&commitment_txid,
-				commitment_data.tx.negotiated_feerate_per_kw(),
-				funding.get_counterparty_selected_contest_delay().unwrap(),
-				&htlc,
-				funding.get_channel_type(),
-				&holder_keys.broadcaster_delayed_payment_key,
-				&holder_keys.revocation_key,
-			);
-
-			let htlc_redeemscript =
-				chan_utils::get_htlc_redeemscript(&htlc, funding.get_channel_type(), &holder_keys);
-			let channel_type = funding.get_channel_type();
-			let htlc_sighashtype = if channel_type.supports_anchors_zero_fee_htlc_tx()
-				|| channel_type.supports_anchor_zero_fee_commitments()
-			{
-				EcdsaSighashType::SinglePlusAnyoneCanPay
-			} else {
-				EcdsaSighashType::All
-			};
-			let htlc_sighash = hash_to_message!(
-				&sighash::SighashCache::new(&htlc_tx)
-					.p2wsh_signature_hash(
-						0,
-						&htlc_redeemscript,
-						htlc.to_bitcoin_amount(),
-						htlc_sighashtype
-					)
-					.unwrap()[..]
-			);
-			log_trace!(logger, "Checking HTLC tx signature {} by key {} against tx {} (sighash {}) with redeemscript {} in channel {}.",
-				log_bytes!(counterparty_sig.serialize_compact()[..]),
-				log_bytes!(holder_keys.countersignatory_htlc_key.to_public_key().serialize()),
-				encode::serialize_hex(&htlc_tx),
-				log_bytes!(htlc_sighash[..]),
-				encode::serialize_hex(&htlc_redeemscript),
-				&self.channel_id(),
-			);
-			if let Err(_) = self.secp_ctx.verify_ecdsa(
-				&htlc_sighash,
-				&counterparty_sig,
-				&holder_keys.countersignatory_htlc_key.to_public_key(),
-			) {
-				return Err(ChannelError::close("Invalid HTLC tx signature from peer".to_owned()));
-			}
-		}
-
 		let holder_commitment_tx = HolderCommitmentTransaction::new(
 			commitment_data.tx,
 			msg.signature,
@@ -6165,8 +6105,10 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 
 		self.holder_signer
 			.validate_holder_commitment(
+				&funding.channel_transaction_parameters,
 				&holder_commitment_tx,
 				commitment_data.outbound_htlc_preimages,
+				&self.secp_ctx,
 			)
 			.map_err(|_| ChannelError::close("Failed to validate our commitment".to_owned()))?;
 
@@ -18088,6 +18030,8 @@ mod tests {
 	use crate::types::payment::{PaymentHash, PaymentPreimage};
 	use crate::util::config::UserConfig;
 	use crate::util::errors::APIError;
+	#[cfg(ldk_test_vectors)]
+	use crate::util::logger::Logger;
 	use crate::util::ser::{ReadableArgs, Writeable};
 	use crate::util::test_utils::{
 		self, OnGetShutdownScriptpubkey, TestFeeEstimator, TestKeysInterface, TestLogger,
@@ -18126,20 +18070,20 @@ mod tests {
 	}
 
 	#[cfg(ldk_test_vectors)]
-	struct Keys {
-		signer: crate::sign::InMemorySigner,
+	struct Keys<'a> {
+		signer: crate::sign::InMemorySigner<&'a TestLogger>,
 	}
 
 	#[cfg(ldk_test_vectors)]
-	impl EntropySource for Keys {
+	impl EntropySource for Keys<'_> {
 		fn get_secure_random_bytes(&self) -> [u8; 32] {
 			[0; 32]
 		}
 	}
 
 	#[cfg(ldk_test_vectors)]
-	impl SignerProvider for Keys {
-		type EcdsaSigner = InMemorySigner;
+	impl<'a> SignerProvider for Keys<'a> {
+		type EcdsaSigner = InMemorySigner<&'a TestLogger>;
 
 		fn generate_channel_keys_id(&self, _inbound: bool, _user_channel_id: u128) -> [u8; 32] {
 			self.signer.channel_keys_id()
@@ -19052,9 +18996,7 @@ mod tests {
 		use crate::ln::channel::{HTLCOutputInCommitment, PredictedNextFee};
 		use crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint};
 		use crate::sign::{ecdsa::EcdsaChannelSigner, ChannelDerivationParameters, HTLCDescriptor};
-		use crate::sync::Arc;
 		use crate::types::payment::PaymentPreimage;
-		use crate::util::logger::Logger;
 		use crate::util::test_utils::{
 			preimage_from_hex, pubkey_from_hex, public_from_secret_hex, secret_from_hex,
 		};
@@ -19069,7 +19011,7 @@ mod tests {
 
 		// Test vectors from BOLT 3 Appendices C and F (anchors):
 		let feeest = TestFeeEstimator::new(15000);
-		let logger: Arc<dyn Logger> = Arc::new(TestLogger::new());
+		let logger = TestLogger::new();
 		let secp_ctx = Secp256k1::new();
 
 		let signer = InMemorySigner::new(
@@ -19088,6 +19030,7 @@ mod tests {
 			],
 			[0; 32],
 			[0; 32],
+			&logger,
 		);
 
 		let holder_pubkeys = signer.pubkeys(&secp_ctx);
@@ -19114,7 +19057,7 @@ mod tests {
 			0,
 			42,
 			None,
-			&*logger,
+			&logger,
 			None,
 		)
 		.unwrap(); // Nothing uses their network key in this test
@@ -19742,11 +19685,9 @@ mod tests {
 		};
 		use crate::ln::channel::HTLCOutputInCommitment;
 		use crate::sign::{ecdsa::EcdsaChannelSigner, ChannelDerivationParameters, HTLCDescriptor};
-		use crate::sync::Arc;
 		use crate::types::features::ChannelTypeFeatures;
 		use crate::types::payment::PaymentPreimage;
 		use crate::util::config::UserConfig;
-		use crate::util::logger::Logger;
 		use crate::util::test_utils::{
 			payment_hash_from_hex, preimage_from_hex, pubkey_from_hex, secret_from_hex,
 		};
@@ -19759,7 +19700,7 @@ mod tests {
 		use core::str::FromStr;
 
 		let feeest = TestFeeEstimator::new(250); // Fee doesn't matter
-		let logger: Arc<dyn Logger> = Arc::new(TestLogger::new());
+		let logger = TestLogger::new();
 		let secp_ctx = Secp256k1::new();
 
 		let alice_funding_privkey =
@@ -19786,6 +19727,7 @@ mod tests {
 			[0xff; 32],
 			[0; 32],
 			[0; 32],
+			&logger,
 		);
 		let alice_keys_provider = Keys { signer: alice_signer.clone() };
 		let alice_pubkeys = alice_signer.pubkeys(&secp_ctx);
@@ -19813,6 +19755,7 @@ mod tests {
 			[0xff; 32],
 			[0; 32],
 			[0; 32],
+			&logger,
 		);
 
 		// Test vectors only provide revocation_basepoint for bob, override it here.
@@ -19839,7 +19782,7 @@ mod tests {
 			0,
 			0,
 			None,
-			&*logger,
+			&logger,
 			None,
 		)
 		.unwrap();

### lightning/src/ln/channel_open_tests.rs
@@ -1469,7 +1469,7 @@ pub fn test_duplicate_funding_err_in_funding() {
 	funding_created_msg.funding_output_index += 10;
 	nodes[1].node.handle_funding_created(node_c_id, &funding_created_msg);
 	get_err_msg(&nodes[1], &node_c_id);
-	let err = "Invalid funding_created signature from peer".to_owned();
+	let err = "Failed to validate our commitment".to_owned();
 	let reason = ClosureReason::ProcessingError { err };
 	let expected_closing = ExpectedCloseEvent::from_id_reason(real_channel_id, false, reason);
 	check_closed_events(&nodes[1], &[expected_closing]);
@@ -2476,6 +2476,47 @@ pub fn test_manual_funding_abandon() {
 	}));
 }
 
+#[xtest(feature = "_externalize_tests")]
+pub fn test_invalid_funding_signed_signature() {
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let node_a_id = nodes[0].node.get_our_node_id();
+	let node_b_id = nodes[1].node.get_our_node_id();
+	let temporary_channel_id = exchange_open_accept_chan(&nodes[0], &nodes[1], 100_000, 0);
+	let (funding_temporary_channel_id, funding_tx, funding_outpoint) =
+		create_funding_transaction(&nodes[0], &node_b_id, 100_000, 42);
+	assert_eq!(temporary_channel_id, funding_temporary_channel_id);
+
+	nodes[0]
+		.node
+		.funding_transaction_generated(funding_temporary_channel_id, node_b_id, funding_tx)
+		.unwrap();
+	check_added_monitors(&nodes[0], 0);
+
+	let funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, node_b_id);
+	nodes[1].node.handle_funding_created(node_a_id, &funding_created);
+	check_added_monitors(&nodes[1], 1);
+	let channel_id = expect_channel_pending_event(&nodes[1], &node_a_id);
+	assert_eq!(channel_id, ChannelId::v1_from_funding_outpoint(funding_outpoint));
+
+	let mut funding_signed =
+		get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, node_a_id);
+	corrupt_signature(&mut funding_signed.signature);
+	nodes[0].node.handle_funding_signed(node_b_id, &funding_signed);
+
+	check_added_monitors(&nodes[0], 0);
+	assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty());
+	assert!(nodes[0].node.list_channels().is_empty());
+	let error_message = get_err_msg(&nodes[0], &node_b_id);
+	assert_eq!(error_message.data, "Failed to validate our commitment");
+	let reason =
+		ClosureReason::ProcessingError { err: "Failed to validate our commitment".to_owned() };
+	check_closed_events(&nodes[0], &[ExpectedCloseEvent::from_id_reason(channel_id, true, reason)]);
+}
+
 #[xtest(feature = "_externalize_tests")]
 pub fn test_funding_signed_event() {
 	let mut cfg = UserConfig::default();

### lightning/src/ln/channelmanager.rs
@@ -2071,21 +2071,21 @@ struct PendingInboundPayment {
 pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<
 	Arc<M>,
 	Arc<T>,
-	Arc<KeysManager>,
-	Arc<KeysManager>,
-	Arc<KeysManager>,
+	Arc<KeysManager<Arc<L>>>,
+	Arc<KeysManager<Arc<L>>>,
+	Arc<KeysManager<Arc<L>>>,
 	Arc<F>,
 	Arc<
 		DefaultRouter<
 			Arc<NetworkGraph<Arc<L>>>,
 			Arc<L>,
-			Arc<KeysManager>,
+			Arc<KeysManager<Arc<L>>>,
 			Arc<RwLock<ProbabilisticScorer<Arc<NetworkGraph<Arc<L>>>, Arc<L>>>>,
 			ProbabilisticScoringFeeParameters,
 			ProbabilisticScorer<Arc<NetworkGraph<Arc<L>>>, Arc<L>>,
 		>,
 	>,
-	Arc<DefaultMessageRouter<Arc<NetworkGraph<Arc<L>>>, Arc<L>, Arc<KeysManager>>>,
+	Arc<DefaultMessageRouter<Arc<NetworkGraph<Arc<L>>>, Arc<L>, Arc<KeysManager<Arc<L>>>>>,
 	Arc<L>,
 >;
 
@@ -2104,19 +2104,19 @@ pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<
 pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, 'i, M, T, F, L> = ChannelManager<
 	&'a M,
 	&'b T,
-	&'c KeysManager,
-	&'c KeysManager,
-	&'c KeysManager,
+	&'c KeysManager<&'g L>,
+	&'c KeysManager<&'g L>,
+	&'c KeysManager<&'g L>,
 	&'d F,
 	&'e DefaultRouter<
 		&'f NetworkGraph<&'g L>,
 		&'g L,
-		&'c KeysManager,
+		&'c KeysManager<&'g L>,
 		&'h RwLock<ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>>,
 		ProbabilisticScoringFeeParameters,
 		ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>,
 	>,
-	&'i DefaultMessageRouter<&'f NetworkGraph<&'g L>, &'g L, &'c KeysManager>,
+	&'i DefaultMessageRouter<&'f NetworkGraph<&'g L>, &'g L, &'c KeysManager<&'g L>>,
 	&'g L,
 >;
 
@@ -2248,7 +2248,7 @@ impl<
 /// use lightning::util::config::UserConfig;
 /// use lightning::util::ser::ReadableArgs;
 ///
-/// # fn read_channel_monitors() -> Vec<ChannelMonitor<lightning::sign::InMemorySigner>> { vec![] }
+/// # fn read_channel_monitors<'a, L: lightning::util::logger::Logger>() -> Vec<ChannelMonitor<lightning::sign::InMemorySigner<&'a L>>> { vec![] }
 /// # fn example<
 /// #     'a,
 /// #     L: lightning::util::logger::Logger,
@@ -2259,14 +2259,14 @@ impl<
 /// #     R: lightning::io::Read,
 /// # >(
 /// #     fee_estimator: &dyn lightning::chain::chaininterface::FeeEstimator,
-/// #     chain_monitor: &dyn lightning::chain::Watch<lightning::sign::InMemorySigner>,
+/// #     chain_monitor: &dyn lightning::chain::Watch<lightning::sign::InMemorySigner<&'a L>>,
 /// #     tx_broadcaster: &dyn lightning::chain::chaininterface::BroadcasterInterface,
 /// #     router: &lightning::routing::router::DefaultRouter<&NetworkGraph<&'a L>, &'a L, &ES, &S, SP, SL>,
 /// #     message_router: &lightning::onion_message::messenger::DefaultMessageRouter<&NetworkGraph<&'a L>, &'a L, &ES>,
 /// #     logger: &L,
 /// #     entropy_source: &ES,
 /// #     node_signer: &dyn lightning::sign::NodeSigner,
-/// #     signer_provider: &lightning::sign::DynSignerProvider,
+/// #     signer_provider: &lightning::sign::DynSignerProvider<&'a L>,
 /// #     best_block: lightning::chain::BlockLocator,
 /// #     current_timestamp: u32,
 /// #     mut reader: R,
@@ -22661,36 +22661,45 @@ pub mod bench {
 
 	use criterion::Criterion;
 
-	type Manager<'a, P> = ChannelManager<
+	type Manager<'a, 'logger, P> = ChannelManager<
 		&'a ChainMonitor<
-			InMemorySigner,
+			InMemorySigner<&'logger test_utils::TestLogger>,
 			&'a test_utils::TestChainSource,
 			&'a test_utils::TestBroadcaster,
 			&'a test_utils::TestFeeEstimator,
 			&'a test_utils::TestLogger,
 			&'a P,
-			&'a KeysManager,
+			&'a KeysManager<&'logger test_utils::TestLogger>,
 		>,
 		&'a test_utils::TestBroadcaster,
-		&'a KeysManager,
-		&'a KeysManager,
-		&'a KeysManager,
+		&'a KeysManager<&'logger test_utils::TestLogger>,
+		&'a KeysManager<&'logger test_utils::TestLogger>,
+		&'a KeysManager<&'logger test_utils::TestLogger>,
 		&'a test_utils::TestFeeEstimator,
 		&'a test_utils::TestRouter<'a>,
 		&'a test_utils::TestMessageRouter<'a>,
 		&'a test_utils::TestLogger,
 	>;
 
-	struct ANodeHolder<'node_cfg, 'chan_mon_cfg: 'node_cfg, P: Persist<InMemorySigner>> {
-		node: &'node_cfg Manager<'chan_mon_cfg, P>,
-	}
-	impl<'node_cfg, 'chan_mon_cfg: 'node_cfg, P: Persist<InMemorySigner>> NodeHolder
-		for ANodeHolder<'node_cfg, 'chan_mon_cfg, P>
+	struct ANodeHolder<
+		'node_cfg,
+		'chan_mon_cfg: 'node_cfg,
+		'logger: 'chan_mon_cfg,
+		P: Persist<InMemorySigner<&'logger test_utils::TestLogger>>,
+	> {
+		node: &'node_cfg Manager<'chan_mon_cfg, 'logger, P>,
+	}
+	impl<
+			'node_cfg,
+			'chan_mon_cfg: 'node_cfg,
+			'logger: 'chan_mon_cfg,
+			P: Persist<InMemorySigner<&'logger test_utils::TestLogger>>,
+		> NodeHolder for ANodeHolder<'node_cfg, 'chan_mon_cfg, 'logger, P>
 	{
-		type CM = Manager<'chan_mon_cfg, P>;
+		type CM = Manager<'chan_mon_cfg, 'logger, P>;
 		#[inline]
 		#[rustfmt::skip]
-		fn node(&self) -> &Manager<'chan_mon_cfg, P> { self.node }
+		fn node(&self) -> &Manager<'chan_mon_cfg, 'logger, P> { self.node }
 		#[inline]
 		#[rustfmt::skip]
 		fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor> { None }
@@ -22702,7 +22711,7 @@ pub mod bench {
 	}
 
 	#[rustfmt::skip]
-	pub fn bench_two_sends<P: Persist<InMemorySigner>>(bench: &mut Criterion, bench_name: &str, persister_a: P, persister_b: P) {
+	pub fn bench_two_sends<P: for<'logger> Persist<InMemorySigner<&'logger test_utils::TestLogger>>>(bench: &mut Criterion, bench_name: &str, persister_a: P, persister_b: P) {
 		// Do a simple benchmark of sending a payment back and forth between two nodes.
 		// Note that this is unrealistic as each payment send will require at least two fsync
 		// calls per node.
@@ -22722,8 +22731,8 @@ pub mod bench {
 		config.channel_handshake_config.minimum_depth = 1;
 
 		let seed_a = [1u8; 32];
-		let keys_manager_a = KeysManager::new(&seed_a, 42, 42, true);
-		let chain_monitor_a = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_a, &keys_manager_a, keys_manager_a.get_peer_storage_key(), false);
+		let keys_manager_a = KeysManager::new(&seed_a, 42, 42, true, &logger_a);
+		let chain_monitor_a = ChainMonitor::new(None::<&test_utils::TestChainSource>, &tx_broadcaster, &logger_a, &fee_estimator, &persister_a, &keys_manager_a, keys_manager_a.get_peer_storage_key(), false);
 		let node_a = ChannelManager::new(&fee_estimator, &chain_monitor_a, &tx_broadcaster, &router, &message_router, &logger_a, &keys_manager_a, &keys_manager_a, &keys_manager_a, config.clone(), ChainParameters {
 			network,
 			best_block: BlockLocator::from_network(network),
@@ -22732,8 +22741,8 @@ pub mod bench {
 
 		let logger_b = test_utils::TestLogger::with_id("node a".to_owned());
 		let seed_b = [2u8; 32];
-		let keys_manager_b = KeysManager::new(&seed_b, 42, 42, true);
-		let chain_monitor_b = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_b, &keys_manager_b, keys_manager_b.get_peer_storage_key(), false);
+		let keys_manager_b = KeysManager::new(&seed_b, 42, 42, true, &logger_b);
+		let chain_monitor_b = ChainMonitor::new(None::<&test_utils::TestChainSource>, &tx_broadcaster, &logger_a, &fee_estimator, &persister_b, &keys_manager_b, keys_manager_b.get_peer_storage_key(), false);
 		let node_b = ChannelManager::new(&fee_estimator, &chain_monitor_b, &tx_broadcaster, &router, &message_router, &logger_b, &keys_manager_b, &keys_manager_b, &keys_manager_b, config.clone(), ChainParameters {
 			network,
 			best_block: BlockLocator::from_network(network),

### lightning/src/ln/functional_test_utils.rs
@@ -68,7 +68,7 @@ use bitcoin::network::Network;
 use bitcoin::policy::MAX_STANDARD_TX_WEIGHT;
 use bitcoin::pow::CompactTarget;
 use bitcoin::script::ScriptBuf;
-use bitcoin::secp256k1::{PublicKey, SecretKey};
+use bitcoin::secp256k1::{ecdsa::Signature, PublicKey, SecretKey};
 use bitcoin::transaction::{self, Version as TxVersion};
 use bitcoin::transaction::{Transaction, TxIn, TxOut};
 use bitcoin::OutPoint as BitcoinOutPoint;
@@ -85,6 +85,12 @@ use core::ops::Deref;
 
 pub const CHAN_CONFIRM_DEPTH: u32 = 10;
 
+pub fn corrupt_signature(signature: &mut Signature) {
+	let mut bytes = signature.serialize_compact();
+	bytes[0] ^= 1;
+	*signature = Signature::from_compact(&bytes).unwrap();
+}
+
 /// Mine the given transaction in the next block and then mine CHAN_CONFIRM_DEPTH - 1 blocks on
 /// top, giving the given transaction CHAN_CONFIRM_DEPTH confirmations.
 ///

### lightning/src/ln/functional_tests.rs
@@ -80,6 +80,54 @@ use lightning_macros::xtest;
 
 use crate::ln::functional_test_utils::*;
 
+#[xtest(feature = "_externalize_tests")]
+pub fn test_invalid_holder_commitment_signatures() {
+	do_test_invalid_holder_commitment_signature(false);
+	do_test_invalid_holder_commitment_signature(true);
+}
+
+fn do_test_invalid_holder_commitment_signature(corrupt_htlc_signature: bool) {
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let node_id_0 = nodes[0].node.get_our_node_id();
+	let node_id_1 = nodes[1].node.get_our_node_id();
+	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+
+	let payment_amount = 1_000_000;
+	let (route, payment_hash, _payment_preimage, payment_secret) =
+		get_route_and_payment_hash!(&nodes[0], &nodes[1], payment_amount);
+	let onion = RecipientOnionFields::secret_only(payment_secret, payment_amount);
+	let payment_id = PaymentId(payment_hash.0);
+	nodes[0].node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();
+	check_added_monitors(&nodes[0], 1);
+
+	let mut update = get_htlc_update_msgs(&nodes[0], &node_id_1);
+	assert_eq!(update.update_add_htlcs.len(), 1);
+	assert_eq!(update.commitment_signed.len(), 1);
+	assert_eq!(update.commitment_signed[0].htlc_signatures.len(), 1);
+	nodes[1].node.handle_update_add_htlc(node_id_0, &update.update_add_htlcs[0]);
+	if corrupt_htlc_signature {
+		corrupt_signature(&mut update.commitment_signed[0].htlc_signatures[0]);
+	} else {
+		corrupt_signature(&mut update.commitment_signed[0].signature);
+	}
+	nodes[1].node.handle_commitment_signed(node_id_0, &update.commitment_signed[0]);
+
+	check_added_monitors(&nodes[1], 1);
+	let error_messages = check_closed_broadcast(&nodes[1], 1, true);
+	assert_eq!(error_messages.len(), 1);
+	assert_eq!(error_messages[0].data, "Failed to validate our commitment");
+	let reason =
+		ClosureReason::ProcessingError { err: "Failed to validate our commitment".to_owned() };
+	check_closed_events(
+		&nodes[1],
+		&[ExpectedCloseEvent::from_id_reason(channel_id, false, reason)],
+	);
+}
+
 #[xtest(feature = "_externalize_tests")]
 pub fn fake_network_test() {
 	// Simple test which builds a network of ChannelManagers, connects them to each other, and

### lightning/src/ln/invoice_utils.rs
@@ -600,10 +600,11 @@ mod test {
 	use crate::ln::outbound_payment::{RecipientOnionFields, Retry};
 	use crate::routing::router::{PaymentParameters, RouteParameters, RouteParametersConfig};
 	use crate::sign::PhantomKeysManager;
+	use crate::sync::Arc;
 	use crate::types::payment::{PaymentHash, PaymentPreimage};
 	use crate::util::config::UserConfig;
 	use crate::util::dyn_signer::{DynKeysInterface, DynPhantomKeysInterface};
-	use crate::util::test_utils;
+	use crate::util::test_utils::{self, TestLogger};
 	use bitcoin::hashes::sha256::Hash as Sha256;
 	use bitcoin::hashes::Hash;
 	use bitcoin::network::Network;
@@ -1201,7 +1202,14 @@ mod test {
 
 	fn make_dyn_keys_interface(seed: &[u8; 32]) -> DynKeysInterface {
 		let cross_node_seed = [44u8; 32];
-		let inner = PhantomKeysManager::new(&seed, 43, 44, &cross_node_seed, true);
+		let inner = PhantomKeysManager::new(
+			&seed,
+			43,
+			44,
+			&cross_node_seed,
+			true,
+			Arc::new(TestLogger::new()),
+		);
 		let dyn_inner = DynPhantomKeysInterface::new(inner);
 		DynKeysInterface::new(Box::new(dyn_inner))
 	}

### lightning/src/ln/onion_payment.rs
@@ -781,9 +781,10 @@ mod tests {
 		// adding an intermediate onion layer, causing the receiver to error with "final payload
 		// provided for us as an intermediate node."
 		let secp_ctx = Secp256k1::new();
-		let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42, true);
+		let logger = test_utils::TestLogger::new();
+		let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42, true, &logger);
 		let bob_pk = PublicKey::from_secret_key(&secp_ctx, &bob.get_node_secret_key());
-		let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42, true);
+		let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42, true, &logger);
 		let charlie_pk = PublicKey::from_secret_key(&secp_ctx, &charlie.get_node_secret_key());
 
 		let (
@@ -810,10 +811,11 @@ mod tests {
 	fn test_peel_payment_onion() {
 		use super::*;
 		let secp_ctx = Secp256k1::new();
+		let logger = test_utils::TestLogger::new();
 
-		let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42, true);
+		let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42, true, &logger);
 		let bob_pk = PublicKey::from_secret_key(&secp_ctx, &bob.get_node_secret_key());
-		let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42, true);
+		let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42, true, &logger);
 		let charlie_pk = PublicKey::from_secret_key(&secp_ctx, &charlie.get_node_secret_key());
 
 		let (session_priv, total_amt_msat, cur_height, recipient_onion, preimage, payment_hash,

### lightning/src/ln/our_peer_storage.rs
@@ -36,8 +36,14 @@ use crate::prelude::*;
 /// ```
 /// use lightning::ln::our_peer_storage::DecryptedOurPeerStorage;
 /// use lightning::sign::{KeysManager, NodeSigner};
+/// use lightning::util::logger::{Logger, Record};
+/// struct FakeLogger;
+/// impl Logger for FakeLogger {
+///     fn log(&self, record: Record) { println!("{:?}", record); }
+/// }
+/// let logger = FakeLogger;
 /// let seed = [1u8; 32];
-/// let keys_mgr = KeysManager::new(&seed, 42, 42, true);
+/// let keys_mgr = KeysManager::new(&seed, 42, 42, true, &logger);
 /// let key = keys_mgr.get_peer_storage_key();
 /// let decrypted_ops = DecryptedOurPeerStorage::new(vec![1, 2, 3]);
 /// let our_peer_storage = decrypted_ops.encrypt(&key, &[0u8; 32]);

### lightning/src/ln/peer_handler.rs
@@ -944,8 +944,18 @@ pub type SimpleArcPeerManager<SD, M, T, F, C, L, CF, S> = PeerManager<
 	Arc<SimpleArcOnionMessenger<M, T, F, L>>,
 	Arc<L>,
 	IgnoringMessageHandler,
-	Arc<KeysManager>,
-	Arc<ChainMonitor<InMemorySigner, Arc<CF>, Arc<T>, Arc<F>, Arc<L>, Arc<S>, Arc<KeysManager>>>,
+	Arc<KeysManager<Arc<L>>>,
+	Arc<
+		ChainMonitor<
+			InMemorySigner<Arc<L>>,
+			Arc<CF>,
+			Arc<T>,
+			Arc<F>,
+			Arc<L>,
+			Arc<S>,
+			Arc<KeysManager<Arc<L>>>,
+		>,
+	>,
 >;
 
 /// SimpleRefPeerManager is a type alias for a PeerManager reference, and is the reference
@@ -967,8 +977,16 @@ pub type SimpleRefPeerManager<
 	&'h SimpleRefOnionMessenger<'a, 'b, 'c, 'd, 'e, 'graph, 'logger, 'i, 'j, 'k, M, T, F, L>,
 	&'logger L,
 	IgnoringMessageHandler,
-	&'c KeysManager,
-	&'j ChainMonitor<&'a M, C, &'b T, &'c F, &'logger L, &'c KeysManager, &'c KeysManager>,
+	&'c KeysManager<&'logger L>,
+	&'j ChainMonitor<
+		&'a M,
+		C,
+		&'b T,
+		&'c F,
+		&'logger L,
+		&'c KeysManager<&'logger L>,
+		&'c KeysManager<&'logger L>,
+	>,
 >;
 
 /// A generic trait which is implemented for all [`PeerManager`]s. This makes bounding functions or

### lightning/src/ln/splicing_tests.rs
@@ -43,7 +43,6 @@ use crate::util::wallet_utils::{
 use crate::sync::Arc;
 
 use bitcoin::hashes::Hash;
-use bitcoin::secp256k1::ecdsa::Signature;
 use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
 use bitcoin::transaction::Version;
 use bitcoin::SignedAmount;
@@ -5777,11 +5776,7 @@ fn test_splice_buffer_invalid_commitment_signed_closes_channel() {
 
 	// Invalidate the signature by modifying one byte. This will cause signature verification
 	// to fail when the buffered message is processed.
-	let original_sig = acceptor_commit_sig.commitment_signed[0].signature;
-	let mut sig_bytes = original_sig.serialize_compact();
-	sig_bytes[0] ^= 0x01; // Flip a bit to corrupt the signature
-	acceptor_commit_sig.commitment_signed[0].signature =
-		Signature::from_compact(&sig_bytes).unwrap();
+	corrupt_signature(&mut acceptor_commit_sig.commitment_signed[0].signature);
 
 	// Deliver the acceptor's invalid commitment_signed to the initiator BEFORE the initiator has
 	// called funding_transaction_signed. The message should be buffered, not processed.
@@ -5830,7 +5825,7 @@ fn test_splice_buffer_invalid_commitment_signed_closes_channel() {
 			action: msgs::ErrorAction::SendErrorMessage { ref msg },
 			..
 		} => {
-			assert!(msg.data.contains("Invalid commitment tx signature from peer"));
+			assert_eq!(msg.data, "Failed to validate our commitment");
 		},
 		_ => panic!("Expected HandleError with SendErrorMessage, got {:?}", msg_events[1]),
 	}
@@ -5841,7 +5836,7 @@ fn test_splice_buffer_invalid_commitment_signed_closes_channel() {
 		_ => panic!("Expected BroadcastChannelUpdate, got {:?}", msg_events[2]),
 	}
 
-	let err = "Invalid commitment tx signature from peer".to_owned();
+	let err = "Failed to validate our commitment".to_owned();
 	let reason = ClosureReason::ProcessingError { err };
 	check_closed_events(
 		&nodes[0],
@@ -5850,6 +5845,67 @@ fn test_splice_buffer_invalid_commitment_signed_closes_channel() {
 	check_added_monitors(&nodes[0], 1);
 }
 
+#[test]
+fn test_splice_batched_invalid_holder_commitment_signatures() {
+	// While a splice is pending, updates contain commitments for both the original and candidate
+	// funding scopes. Check the commitment and HTLC signature on each scope independently.
+	for funding_scope in 0..2 {
+		do_test_splice_batched_invalid_holder_commitment_signature(funding_scope, false);
+		do_test_splice_batched_invalid_holder_commitment_signature(funding_scope, true);
+	}
+}
+
+#[cfg(test)]
+fn do_test_splice_batched_invalid_holder_commitment_signature(
+	funding_scope: usize, corrupt_htlc_signature: bool,
+) {
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let node_id_0 = nodes[0].node.get_our_node_id();
+	let node_id_1 = nodes[1].node.get_our_node_id();
+	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+	let outputs = vec![TxOut {
+		value: Amount::from_sat(1_000),
+		script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
+	}];
+	let contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs).unwrap();
+	let _ = splice_channel(&nodes[0], &nodes[1], channel_id, contribution);
+
+	let payment_amount = 1_000_000;
+	let (route, payment_hash, _payment_preimage, payment_secret) =
+		get_route_and_payment_hash!(&nodes[0], &nodes[1], payment_amount);
+	let onion = RecipientOnionFields::secret_only(payment_secret, payment_amount);
+	let payment_id = PaymentId(payment_hash.0);
+	nodes[0].node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();
+	check_added_monitors(&nodes[0], 1);
+
+	let mut update = get_htlc_update_msgs(&nodes[0], &node_id_1);
+	assert_eq!(update.update_add_htlcs.len(), 1);
+	assert_eq!(update.commitment_signed.len(), 2);
+	assert!(update.commitment_signed.iter().all(|msg| msg.htlc_signatures.len() == 1));
+	nodes[1].node.handle_update_add_htlc(node_id_0, &update.update_add_htlcs[0]);
+	if corrupt_htlc_signature {
+		corrupt_signature(&mut update.commitment_signed[funding_scope].htlc_signatures[0]);
+	} else {
+		corrupt_signature(&mut update.commitment_signed[funding_scope].signature);
+	}
+	nodes[1].node.handle_commitment_signed_batch_test(node_id_0, &update.commitment_signed);
+
+	check_added_monitors(&nodes[1], 1);
+	let error_messages = check_closed_broadcast(&nodes[1], 1, true);
+	assert_eq!(error_messages.len(), 1);
+	assert_eq!(error_messages[0].data, "Failed to validate our commitment");
+	let reason =
+		ClosureReason::ProcessingError { err: "Failed to validate our commitment".to_owned() };
+	check_closed_events(
+		&nodes[1],
+		&[ExpectedCloseEvent::from_id_reason(channel_id, false, reason)],
+	);
+}
+
 #[test]
 fn test_splice_waits_for_initial_commitment_monitor_update_before_releasing_tx_signatures() {
 	do_splice_waits_for_initial_commitment_monitor_update_before_releasing_tx_signatures(false);

### lightning/src/onion_message/dns_resolution.rs
@@ -727,7 +727,7 @@ mod tests {
 	use super::*;
 
 	#[cfg(feature = "dnssec")]
-	use crate::util::test_utils::pubkey;
+	use crate::util::test_utils::{pubkey, TestLogger};
 
 	#[cfg(feature = "dnssec")]
 	fn dest(b: u8) -> Destination {
@@ -813,7 +813,8 @@ mod tests {
 	#[test]
 	#[cfg(feature = "dnssec")]
 	fn test_expiry() {
-		let keys = crate::sign::KeysManager::new(&[33; 32], 0, 0, true);
+		let logger = TestLogger::new();
+		let keys = crate::sign::KeysManager::new(&[33; 32], 0, 0, true, &logger);
 		let resolver = OMNameResolver::new(42, 42);
 		let name = HumanReadableName::new("user", "example.com").unwrap();
 
@@ -860,7 +861,8 @@ mod tests {
 	#[test]
 	#[cfg(feature = "dnssec")]
 	fn test_dnssec_error() {
-		let keys = crate::sign::KeysManager::new(&[33; 32], 0, 0, true);
+		let logger = TestLogger::new();
+		let keys = crate::sign::KeysManager::new(&[33; 32], 0, 0, true, &logger);
 		let resolver = OMNameResolver::new(42, 42);
 		let name = HumanReadableName::new("user", "example.com").unwrap();
 
@@ -909,7 +911,8 @@ mod tests {
 		// An error only counts against the resolution whose blinded path (context) it was received
 		// over; other resolutions for the same name (queued with a different `PaymentId`, and thus a
 		// different context) are left untouched.
-		let keys = crate::sign::KeysManager::new(&[33; 32], 0, 0, true);
+		let logger = TestLogger::new();
+		let keys = crate::sign::KeysManager::new(&[33; 32], 0, 0, true, &logger);
 		let resolver = OMNameResolver::new(42, 42);
 		let name = HumanReadableName::new("user", "example.com").unwrap();
 

### lightning/src/onion_message/messenger.rs
@@ -185,8 +185,8 @@ impl<
 /// # }
 /// # let seed = [42u8; 32];
 /// # let time = Duration::from_secs(123456);
-/// # let keys_manager = KeysManager::new(&seed, time.as_secs(), time.subsec_nanos(), true);
-/// # let logger = Arc::new(FakeLogger {});
+/// # let logger = FakeLogger {};
+/// # let keys_manager = KeysManager::new(&seed, time.as_secs(), time.subsec_nanos(), true, &logger);
 /// # let node_secret = SecretKey::from_slice(&<Vec<u8>>::from_hex("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap();
 /// # let secp_ctx = Secp256k1::new();
 /// # let hop_node_id1 = PublicKey::from_secret_key(&secp_ctx, &node_secret);
@@ -201,7 +201,7 @@ impl<
 /// // Create the onion messenger. This must use the same `keys_manager` as is passed to your
 /// // ChannelManager.
 /// let onion_messenger = OnionMessenger::new(
-///     &keys_manager, &keys_manager, logger, &node_id_lookup, message_router,
+///     &keys_manager, &keys_manager, &logger, &node_id_lookup, message_router,
 ///     &offers_message_handler, &async_payments_message_handler, &dns_resolution_message_handler,
 ///     &custom_message_handler,
 /// );
@@ -2389,11 +2389,11 @@ impl<
 /// [`SimpleArcPeerManager`]: crate::ln::peer_handler::SimpleArcPeerManager
 #[cfg(not(c_bindings))]
 pub type SimpleArcOnionMessenger<M, T, F, L> = OnionMessenger<
-	Arc<KeysManager>,
-	Arc<KeysManager>,
+	Arc<KeysManager<Arc<L>>>,
+	Arc<KeysManager<Arc<L>>>,
 	Arc<L>,
 	Arc<SimpleArcChannelManager<M, T, F, L>>,
-	Arc<DefaultMessageRouter<Arc<NetworkGraph<Arc<L>>>, Arc<L>, Arc<KeysManager>>>,
+	Arc<DefaultMessageRouter<Arc<NetworkGraph<Arc<L>>>, Arc<L>, Arc<KeysManager<Arc<L>>>>>,
 	Arc<SimpleArcChannelManager<M, T, F, L>>,
 	Arc<SimpleArcChannelManager<M, T, F, L>>,
 	IgnoringMessageHandler,
@@ -2410,11 +2410,11 @@ pub type SimpleArcOnionMessenger<M, T, F, L> = OnionMessenger<
 #[cfg(not(c_bindings))]
 pub type SimpleRefOnionMessenger<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, 'i, 'j, M, T, F, L> =
 	OnionMessenger<
-		&'a KeysManager,
-		&'a KeysManager,
+		&'a KeysManager<&'b L>,
+		&'a KeysManager<&'b L>,
 		&'b L,
 		&'j SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, 'i, M, T, F, L>,
-		&'i DefaultMessageRouter<&'g NetworkGraph<&'b L>, &'b L, &'a KeysManager>,
+		&'i DefaultMessageRouter<&'g NetworkGraph<&'b L>, &'b L, &'a KeysManager<&'b L>>,
 		&'j SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, 'i, M, T, F, L>,
 		&'j SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, 'i, M, T, F, L>,
 		IgnoringMessageHandler,

### lightning/src/sign/mod.rs
@@ -56,6 +56,7 @@ use crate::ln::script::ShutdownScript;
 use crate::offers::invoice::UnsignedBolt12Invoice;
 use crate::types::features::ChannelTypeFeatures;
 use crate::types::payment::PaymentPreimage;
+use crate::util::logger::Logger;
 use crate::util::native_async::MaybeSend;
 use crate::util::ser::{ReadableArgs, Writeable};
 use crate::util::transaction_utils;
@@ -786,6 +787,11 @@ pub trait ChannelSigner {
 	/// Policy checks should be implemented in this function, including checking the amount
 	/// sent to us and checking the HTLCs.
 	///
+	/// Before using `channel_parameters` for validation, implementations of a validating
+	/// signer must verify that it matches the trusted channel state they maintain, including the
+	/// funding keys, value, and outpoint. A validating signer must not trust parameters supplied
+	/// by an untrusted caller.
+	///
 	/// The preimages of outbound HTLCs that were fulfilled since the last commitment are provided.
 	/// A validating signer should ensure that an HTLC output is removed only when the matching
 	/// preimage is provided, or when the value to holder is restored.
@@ -797,8 +803,9 @@ pub trait ChannelSigner {
 	/// closed. If you wish to make this operation asynchronous, you should instead return `Ok(())`
 	/// and pause future signing operations until this validation completes.
 	fn validate_holder_commitment(
-		&self, holder_tx: &HolderCommitmentTransaction,
-		outbound_htlc_preimages: Vec<PaymentPreimage>,
+		&self, channel_parameters: &ChannelTransactionParameters,
+		holder_tx: &HolderCommitmentTransaction, outbound_htlc_preimages: Vec<PaymentPreimage>,
+		secp_ctx: &Secp256k1<secp256k1::All>,
 	) -> Result<(), ()>;
 
 	/// Validate the counterparty's revocation.
@@ -1084,7 +1091,7 @@ impl<T: OutputSpender + ?Sized, O: Deref<Target = T>> OutputSpender for O {
 ///
 /// This is not exported to bindings users as it is not intended for public consumption.
 #[doc(hidden)]
-pub type DynSignerProvider = dyn SignerProvider<EcdsaSigner = InMemorySigner>;
+pub type DynSignerProvider<L> = dyn SignerProvider<EcdsaSigner = InMemorySigner<L>>;
 
 /// A trait that can return signer instances for individual channels.
 ///
@@ -1274,7 +1281,7 @@ pub fn compute_funding_key_tweak(
 ///
 /// This implementation performs no policy checks and is insufficient by itself as
 /// a secure external signer.
-pub struct InMemorySigner {
+pub struct InMemorySigner<L: Logger> {
 	/// Holder secret key in the 2-of-2 multisig script of a channel. This key also backs the
 	/// holder's anchor output in a commitment transaction, if one is present.
 	funding_key: sealed::MaybeTweakedSecretKey,
@@ -1298,9 +1305,11 @@ pub struct InMemorySigner {
 	channel_keys_id: [u8; 32],
 	/// A source of random bytes.
 	entropy_source: RandomBytes,
+	/// A logger.
+	logger: L,
 }
 
-impl PartialEq for InMemorySigner {
+impl<L: Logger> PartialEq for InMemorySigner<L> {
 	fn eq(&self, other: &Self) -> bool {
 		self.funding_key == other.funding_key
 			&& self.revocation_base_key == other.revocation_base_key
@@ -1314,7 +1323,7 @@ impl PartialEq for InMemorySigner {
 	}
 }
 
-impl Clone for InMemorySigner {
+impl<L: Logger + Clone> Clone for InMemorySigner<L> {
 	fn clone(&self) -> Self {
 		Self {
 			funding_key: self.funding_key.clone(),
@@ -1327,18 +1336,19 @@ impl Clone for InMemorySigner {
 			commitment_seed: self.commitment_seed.clone(),
 			channel_keys_id: self.channel_keys_id,
 			entropy_source: RandomBytes::new(self.get_secure_random_bytes()),
+			logger: self.logger.clone(),
 		}
 	}
 }
 
-impl InMemorySigner {
+impl<L: Logger> InMemorySigner<L> {
 	#[cfg(any(feature = "_test_utils", test))]
 	pub fn new(
 		funding_key: SecretKey, revocation_base_key: SecretKey, payment_key_v1: SecretKey,
 		payment_key_v2: SecretKey, v2_remote_key_derivation: bool,
 		delayed_payment_base_key: SecretKey, htlc_base_key: SecretKey, commitment_seed: [u8; 32],
-		channel_keys_id: [u8; 32], rand_bytes_unique_start: [u8; 32],
-	) -> InMemorySigner {
+		channel_keys_id: [u8; 32], rand_bytes_unique_start: [u8; 32], logger: L,
+	) -> InMemorySigner<L> {
 		InMemorySigner {
 			funding_key: sealed::MaybeTweakedSecretKey::from(funding_key),
 			revocation_base_key,
@@ -1350,6 +1360,7 @@ impl InMemorySigner {
 			commitment_seed,
 			channel_keys_id,
 			entropy_source: RandomBytes::new(rand_bytes_unique_start),
+			logger,
 		}
 	}
 
@@ -1358,8 +1369,8 @@ impl InMemorySigner {
 		funding_key: SecretKey, revocation_base_key: SecretKey, payment_key_v1: SecretKey,
 		payment_key_v2: SecretKey, v2_remote_key_derivation: bool,
 		delayed_payment_base_key: SecretKey, htlc_base_key: SecretKey, commitment_seed: [u8; 32],
-		channel_keys_id: [u8; 32], rand_bytes_unique_start: [u8; 32],
-	) -> InMemorySigner {
+		channel_keys_id: [u8; 32], rand_bytes_unique_start: [u8; 32], logger: L,
+	) -> InMemorySigner<L> {
 		InMemorySigner {
 			funding_key: sealed::MaybeTweakedSecretKey::from(funding_key),
 			revocation_base_key,
@@ -1371,6 +1382,7 @@ impl InMemorySigner {
 			commitment_seed,
 			channel_keys_id,
 			entropy_source: RandomBytes::new(rand_bytes_unique_start),
+			logger,
 		}
 	}
 
@@ -1540,13 +1552,13 @@ impl InMemorySigner {
 	}
 }
 
-impl EntropySource for InMemorySigner {
+impl<L: Logger> EntropySource for InMemorySigner<L> {
 	fn get_secure_random_bytes(&self) -> [u8; 32] {
 		self.entropy_source.get_secure_random_bytes()
 	}
 }
 
-impl ChannelSigner for InMemorySigner {
+impl<L: Logger> ChannelSigner for InMemorySigner<L> {
 	fn get_per_commitment_point(
 		&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>,
 	) -> Result<PublicKey, ()> {
@@ -1561,9 +1573,102 @@ impl ChannelSigner for InMemorySigner {
 	}
 
 	fn validate_holder_commitment(
-		&self, _holder_tx: &HolderCommitmentTransaction,
-		_outbound_htlc_preimages: Vec<PaymentPreimage>,
+		&self, channel_parameters: &ChannelTransactionParameters,
+		holder_commitment_tx: &HolderCommitmentTransaction,
+		_outbound_htlc_preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>,
 	) -> Result<(), ()> {
+		let funding_key = self.funding_key(channel_parameters.splice_parent_funding_txid);
+		let funding_pubkey = funding_key.public_key(secp_ctx);
+		let counterparty_funding_pubkey =
+			channel_parameters.counterparty_pubkeys().expect(MISSING_PARAMS_ERR).funding_pubkey;
+		let funding_script =
+			make_funding_redeemscript(&funding_pubkey, &counterparty_funding_pubkey);
+		let channel_value_satoshis = channel_parameters.channel_value_satoshis;
+		let counterparty_selected_delay = channel_parameters
+			.counterparty_parameters
+			.as_ref()
+			.expect(MISSING_PARAMS_ERR)
+			.selected_contest_delay;
+		let channel_type = &channel_parameters.channel_type_features;
+		let commitment_txid = {
+			let trusted_tx = holder_commitment_tx.trust();
+			let bitcoin_tx = trusted_tx.built_transaction();
+			if bitcoin_tx.transaction.output.is_empty() {
+				log_error!(self.logger, "Commitment tx from peer has 0 outputs");
+				return Err(());
+			}
+
+			let sighash = bitcoin_tx.get_sighash_all(&funding_script, channel_value_satoshis);
+
+			log_trace!(self.logger, "Checking commitment tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}.",
+				log_bytes!(holder_commitment_tx.counterparty_sig.serialize_compact()[..]),
+				log_bytes!(counterparty_funding_pubkey.serialize()),
+				bitcoin::consensus::encode::serialize_hex(&bitcoin_tx.transaction),
+				log_bytes!(sighash[..]), bitcoin::consensus::encode::serialize_hex(&funding_script),
+			);
+			if let Err(_) = secp_ctx.verify_ecdsa(
+				&sighash,
+				&holder_commitment_tx.counterparty_sig,
+				&counterparty_funding_pubkey,
+			) {
+				log_error!(self.logger, "Invalid commitment tx signature from peer");
+				return Err(());
+			}
+			bitcoin_tx.txid
+		};
+
+		let holder_keys = holder_commitment_tx.trust().keys();
+		for (htlc, counterparty_sig) in holder_commitment_tx
+			.nondust_htlcs()
+			.iter()
+			.zip(holder_commitment_tx.counterparty_htlc_sigs.iter())
+		{
+			assert!(htlc.transaction_output_index.is_some());
+			let htlc_tx = chan_utils::build_htlc_transaction(
+				&commitment_txid,
+				holder_commitment_tx.negotiated_feerate_per_kw(),
+				counterparty_selected_delay,
+				&htlc,
+				channel_type,
+				&holder_keys.broadcaster_delayed_payment_key,
+				&holder_keys.revocation_key,
+			);
+
+			let htlc_redeemscript =
+				chan_utils::get_htlc_redeemscript(&htlc, channel_type, &holder_keys);
+			let htlc_sighashtype = if channel_type.supports_anchors_zero_fee_htlc_tx()
+				|| channel_type.supports_anchor_zero_fee_commitments()
+			{
+				EcdsaSighashType::SinglePlusAnyoneCanPay
+			} else {
+				EcdsaSighashType::All
+			};
+			let htlc_sighash = hash_to_message!(
+				&sighash::SighashCache::new(&htlc_tx)
+					.p2wsh_signature_hash(
+						0,
+						&htlc_redeemscript,
+						htlc.to_bitcoin_amount(),
+						htlc_sighashtype
+					)
+					.unwrap()[..]
+			);
+			log_trace!(self.logger, "Checking HTLC tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}.",
+				log_bytes!(counterparty_sig.serialize_compact()[..]),
+				log_bytes!(holder_keys.countersignatory_htlc_key.to_public_key().serialize()),
+				bitcoin::consensus::encode::serialize_hex(&htlc_tx),
+				log_bytes!(htlc_sighash[..]),
+				bitcoin::consensus::encode::serialize_hex(&htlc_redeemscript),
+			);
+			if let Err(_) = secp_ctx.verify_ecdsa(
+				&htlc_sighash,
+				&counterparty_sig,
+				&holder_keys.countersignatory_htlc_key.to_public_key(),
+			) {
+				log_error!(self.logger, "Invalid HTLC tx signature from peer");
+				return Err(());
+			}
+		}
 		Ok(())
 	}
 
@@ -1604,7 +1709,7 @@ impl ChannelSigner for InMemorySigner {
 const MISSING_PARAMS_ERR: &'static str =
 	"ChannelTransactionParameters must be populated before signing operations";
 
-impl EcdsaChannelSigner for InMemorySigner {
+impl<L: Logger> EcdsaChannelSigner for InMemorySigner<L> {
 	fn sign_counterparty_commitment(
 		&self, channel_parameters: &ChannelTransactionParameters,
 		commitment_tx: &CommitmentTransaction, _inbound_htlc_preimages: Vec<PaymentPreimage>,
@@ -1980,7 +2085,7 @@ impl EcdsaChannelSigner for InMemorySigner {
 ///
 /// Note that switching between this struct and [`PhantomKeysManager`] will invalidate any
 /// previously issued invoices and attempts to pay previous invoices will fail.
-pub struct KeysManager {
+pub struct KeysManager<L: Logger> {
 	secp_ctx: Secp256k1<secp256k1::All>,
 	node_secret: SecretKey,
 	node_id: PublicKey,
@@ -2002,9 +2107,10 @@ pub struct KeysManager {
 	seed: [u8; 32],
 	starting_time_secs: u64,
 	starting_time_nanos: u32,
+	logger: L,
 }
 
-impl KeysManager {
+impl<L: Logger> KeysManager<L> {
 	/// Constructs a [`KeysManager`] from a 32-byte seed. If the seed is in some way biased (e.g.,
 	/// your CSRNG is busted) this may panic (but more importantly, you will possibly lose funds).
 	/// `starting_time` isn't strictly required to actually be a time, but it must absolutely,
@@ -2029,7 +2135,7 @@ impl KeysManager {
 	/// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
 	pub fn new(
 		seed: &[u8; 32], starting_time_secs: u64, starting_time_nanos: u32,
-		v2_remote_key_derivation: bool,
+		v2_remote_key_derivation: bool, logger: L,
 	) -> Self {
 		// Constants for key derivation path indices used in this function.
 		const NODE_SECRET_INDEX: ChildNumber = ChildNumber::Hardened { index: 0 };
@@ -2122,6 +2228,7 @@ impl KeysManager {
 					seed: *seed,
 					starting_time_secs,
 					starting_time_nanos,
+					logger,
 				};
 				let secp_seed = res.get_secure_random_bytes();
 				res.secp_ctx.seeded_randomize(&secp_seed);
@@ -2180,9 +2287,11 @@ impl KeysManager {
 			.expect("Your RNG is busted")
 			.private_key
 	}
+}
 
+impl<L: Logger + Clone> KeysManager<L> {
 	/// Derive an old [`EcdsaChannelSigner`] containing per-channel secrets based on a key derivation parameters.
-	pub fn derive_channel_keys(&self, params: &[u8; 32]) -> InMemorySigner {
+	pub fn derive_channel_keys(&self, params: &[u8; 32]) -> InMemorySigner<L> {
 		let chan_id = u64::from_be_bytes(params[0..8].try_into().unwrap());
 		let mut unique_start = Sha256::engine();
 		unique_start.input(params);
@@ -2240,6 +2349,7 @@ impl KeysManager {
 			commitment_seed,
 			params.clone(),
 			prng_seed,
+			self.logger.clone(),
 		)
 	}
 
@@ -2254,7 +2364,7 @@ impl KeysManager {
 	pub fn sign_spendable_outputs_psbt<C: Signing>(
 		&self, descriptors: &[&SpendableOutputDescriptor], mut psbt: Psbt, secp_ctx: &Secp256k1<C>,
 	) -> Result<Psbt, ()> {
-		let mut keys_cache: Option<(InMemorySigner, [u8; 32])> = None;
+		let mut keys_cache: Option<(InMemorySigner<L>, [u8; 32])> = None;
 		for outp in descriptors {
 			let get_input_idx = |outpoint: &OutPoint| {
 				psbt.unsigned_tx
@@ -2363,13 +2473,13 @@ impl KeysManager {
 	}
 }
 
-impl EntropySource for KeysManager {
+impl<L: Logger> EntropySource for KeysManager<L> {
 	fn get_secure_random_bytes(&self) -> [u8; 32] {
 		self.entropy_source.get_secure_random_bytes()
 	}
 }
 
-impl NodeSigner for KeysManager {
+impl<L: Logger> NodeSigner for KeysManager<L> {
 	fn get_node_id(&self, recipient: Recipient) -> Result<PublicKey, ()> {
 		match recipient {
 			Recipient::Node => Ok(self.node_id.clone()),
@@ -2432,7 +2542,7 @@ impl NodeSigner for KeysManager {
 	}
 }
 
-impl OutputSpender for KeysManager {
+impl<L: Logger + Clone> OutputSpender for KeysManager<L> {
 	/// Creates a [`Transaction`] which spends the given descriptors to the given outputs, plus an
 	/// output to the given change destination (if sufficient change value remains).
 	///
@@ -2471,8 +2581,8 @@ impl OutputSpender for KeysManager {
 	}
 }
 
-impl SignerProvider for KeysManager {
-	type EcdsaSigner = InMemorySigner;
+impl<L: Logger + Clone> SignerProvider for KeysManager<L> {
+	type EcdsaSigner = InMemorySigner<L>;
 
 	fn generate_channel_keys_id(&self, _inbound: bool, user_channel_id: u128) -> [u8; 32] {
 		let child_idx = self.channel_child_index.fetch_add(1, Ordering::AcqRel);
@@ -2524,23 +2634,23 @@ impl SignerProvider for KeysManager {
 //
 /// Switching between this struct and [`KeysManager`] will invalidate any previously issued
 /// invoices and attempts to pay previous invoices will fail.
-pub struct PhantomKeysManager {
+pub struct PhantomKeysManager<L: Logger> {
 	#[cfg(test)]
-	pub(crate) inner: KeysManager,
+	pub(crate) inner: KeysManager<L>,
 	#[cfg(not(test))]
-	inner: KeysManager,
+	inner: KeysManager<L>,
 	inbound_payment_key: ExpandedKey,
 	phantom_secret: SecretKey,
 	phantom_node_id: PublicKey,
 }
 
-impl EntropySource for PhantomKeysManager {
+impl<L: Logger> EntropySource for PhantomKeysManager<L> {
 	fn get_secure_random_bytes(&self) -> [u8; 32] {
 		self.inner.get_secure_random_bytes()
 	}
 }
 
-impl NodeSigner for PhantomKeysManager {
+impl<L: Logger> NodeSigner for PhantomKeysManager<L> {
 	fn get_node_id(&self, recipient: Recipient) -> Result<PublicKey, ()> {
 		match recipient {
 			Recipient::Node => self.inner.get_node_id(Recipient::Node),
@@ -2599,7 +2709,7 @@ impl NodeSigner for PhantomKeysManager {
 	}
 }
 
-impl OutputSpender for PhantomKeysManager {
+impl<L: Logger + Clone> OutputSpender for PhantomKeysManager<L> {
 	/// See [`OutputSpender::spend_spendable_outputs`] and [`KeysManager::spend_spendable_outputs`]
 	/// for documentation on this method.
 	fn spend_spendable_outputs(
@@ -2618,8 +2728,8 @@ impl OutputSpender for PhantomKeysManager {
 	}
 }
 
-impl SignerProvider for PhantomKeysManager {
-	type EcdsaSigner = InMemorySigner;
+impl<L: Logger + Clone> SignerProvider for PhantomKeysManager<L> {
+	type EcdsaSigner = InMemorySigner<L>;
 
 	fn generate_channel_keys_id(&self, inbound: bool, user_channel_id: u128) -> [u8; 32] {
 		self.inner.generate_channel_keys_id(inbound, user_channel_id)
@@ -2638,7 +2748,7 @@ impl SignerProvider for PhantomKeysManager {
 	}
 }
 
-impl PhantomKeysManager {
+impl<L: Logger> PhantomKeysManager<L> {
 	/// Constructs a [`PhantomKeysManager`] given a 32-byte seed and an additional `cross_node_seed`
 	/// that is shared across all nodes that intend to participate in [phantom node payments]
 	/// together.
@@ -2652,13 +2762,14 @@ impl PhantomKeysManager {
 	/// [phantom node payments]: PhantomKeysManager
 	pub fn new(
 		seed: &[u8; 32], starting_time_secs: u64, starting_time_nanos: u32,
-		cross_node_seed: &[u8; 32], v2_remote_key_derivation: bool,
+		cross_node_seed: &[u8; 32], v2_remote_key_derivation: bool, logger: L,
 	) -> Self {
 		let inner = KeysManager::new(
 			seed,
 			starting_time_secs,
 			starting_time_nanos,
 			v2_remote_key_derivation,
+			logger,
 		);
 		let (inbound_key, phantom_key) = hkdf_extract_expand_twice(
 			b"LDK Inbound and Phantom Payment Key Expansion",
@@ -2674,11 +2785,6 @@ impl PhantomKeysManager {
 		}
 	}
 
-	/// See [`KeysManager::derive_channel_keys`] for documentation on this method.
-	pub fn derive_channel_keys(&self, params: &[u8; 32]) -> InMemorySigner {
-		self.inner.derive_channel_keys(params)
-	}
-
 	/// Gets the "node_id" secret key used to sign gossip announcements, decode onion data, etc.
 	pub fn get_node_secret_key(&self) -> SecretKey {
 		self.inner.get_node_secret_key()
@@ -2691,6 +2797,13 @@ impl PhantomKeysManager {
 	}
 }
 
+impl<L: Logger + Clone> PhantomKeysManager<L> {
+	/// See [`KeysManager::derive_channel_keys`] for documentation on this method.
+	pub fn derive_channel_keys(&self, params: &[u8; 32]) -> InMemorySigner<L> {
+		self.inner.derive_channel_keys(params)
+	}
+}
+
 /// An implementation of [`EntropySource`] using ChaCha20.
 pub struct RandomBytes {
 	/// Seed from which all randomness produced is derived from.
@@ -2792,6 +2905,7 @@ fn sweep_weight_estimate_accounts_for_to_self_delay() {
 #[cfg(ldk_bench)]
 pub mod benches {
 	use crate::sign::{EntropySource, KeysManager};
+	use crate::util::test_utils::TestLogger;
 	use bitcoin::constants::genesis_block;
 	use bitcoin::Network;
 	use std::sync::mpsc::TryRecvError;
@@ -2804,8 +2918,13 @@ pub mod benches {
 	pub fn bench_get_secure_random_bytes(bench: &mut Criterion) {
 		let seed = [0u8; 32];
 		let now = Duration::from_secs(genesis_block(Network::Testnet).header.time as u64);
-		let keys_manager =
-			Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_micros(), true));
+		let keys_manager = Arc::new(KeysManager::new(
+			&seed,
+			now.as_secs(),
+			now.subsec_micros(),
+			true,
+			TestLogger::new(),
+		));
 
 		let mut handles = Vec::new();
 		let mut stops = Vec::new();

### lightning/src/util/dyn_signer.rs
@@ -1,6 +1,7 @@
 //! A dynamically dispatched signer
 
 use crate::prelude::*;
+use crate::sync::Arc;
 
 use core::any::Any;
 
@@ -18,6 +19,8 @@ use crate::sign::{EntropySource, HTLCDescriptor, OutputSpender, PhantomKeysManag
 use crate::sign::{
 	NodeSigner, PeerStorageKey, Recipient, SignerProvider, SpendableOutputDescriptor,
 };
+use crate::util::logger::Logger;
+use crate::util::test_utils::TestLogger;
 use bitcoin;
 use bitcoin::absolute::LockTime;
 use bitcoin::secp256k1::All;
@@ -101,8 +104,10 @@ delegate!(DynSigner, ChannelSigner,
 	) -> Result<PublicKey, ()>,
 	fn release_commitment_secret(, idx: u64) -> Result<[u8; 32], ()>,
 	fn validate_holder_commitment(,
+		channel_parameters: &ChannelTransactionParameters,
 		holder_tx: &HolderCommitmentTransaction,
-		preimages: Vec<PaymentPreimage>
+		preimages: Vec<PaymentPreimage>,
+		secp_ctx: &Secp256k1<secp256k1::All>
 	) -> Result<(), ()>,
 	fn pubkeys(,
 		secp_ctx: &Secp256k1<secp256k1::All>
@@ -114,9 +119,9 @@ delegate!(DynSigner, ChannelSigner,
 	fn validate_counterparty_revocation(, idx: u64, secret: &SecretKey) -> Result<(), ()>
 );
 
-impl DynSignerTrait for InMemorySigner {}
+impl<L: Logger + Clone + Send + Sync + 'static> DynSignerTrait for InMemorySigner<L> {}
 
-impl InnerSign for InMemorySigner {
+impl<L: Logger + Clone + Send + Sync + 'static> InnerSign for InMemorySigner<L> {
 	fn box_clone(&self) -> Box<dyn InnerSign> {
 		Box::new(self.clone())
 	}
@@ -182,12 +187,12 @@ pub trait DynKeysInterfaceTrait:
 
 /// A dyn wrapper for PhantomKeysManager
 pub struct DynPhantomKeysInterface {
-	inner: Box<PhantomKeysManager>,
+	inner: Box<PhantomKeysManager<Arc<TestLogger>>>,
 }
 
 impl DynPhantomKeysInterface {
 	/// Create a new DynPhantomKeysInterface
-	pub fn new(inner: PhantomKeysManager) -> Self {
+	pub fn new(inner: PhantomKeysManager<Arc<TestLogger>>) -> Self {
 		DynPhantomKeysInterface { inner: Box::new(inner) }
 	}
 }

### lightning/src/util/test_channel_signer.rs
@@ -43,8 +43,9 @@ use bitcoin::secp256k1::{PublicKey, SecretKey};
 /// Initial value for revoked commitment downward counter
 pub const INITIAL_REVOKED_COMMITMENT_NUMBER: u64 = 1 << 48;
 
-/// An implementation of Sign that enforces some policy checks.  The current checks
-/// are an incomplete set.  They include:
+/// An implementation of ChannelSigner that enforces some policy checks in addition to checking
+/// counterparty signatures on the commitment and HTLC transactions. The current policy
+/// checks are an incomplete set. They include:
 ///
 /// - When signing, the holder transaction has not been revoked
 /// - When revoking, the holder transaction has not been signed
@@ -57,9 +58,6 @@ pub const INITIAL_REVOKED_COMMITMENT_NUMBER: u64 = 1 << 48;
 /// Eventually we will probably want to expose a variant of this which would essentially
 /// be what you'd want to run on a hardware wallet.
 ///
-/// Note that counterparty signatures on the holder transaction are not checked, but it should
-/// be in a complete implementation.
-///
 /// Note that before we do so we should ensure its serialization format has backwards- and
 /// forwards-compatibility prefix/suffixes!
 pub struct TestChannelSigner {
@@ -210,8 +208,9 @@ impl ChannelSigner for TestChannelSigner {
 	}
 
 	fn validate_holder_commitment(
-		&self, holder_tx: &HolderCommitmentTransaction,
-		_outbound_htlc_preimages: Vec<PaymentPreimage>,
+		&self, channel_parameters: &ChannelTransactionParameters,
+		holder_tx: &HolderCommitmentTransaction, outbound_htlc_preimages: Vec<PaymentPreimage>,
+		secp_ctx: &Secp256k1<secp256k1::All>,
 	) -> Result<(), ()> {
 		let mut state = self.state.lock().unwrap();
 		let idx = holder_tx.commitment_number();
@@ -223,6 +222,14 @@ impl ChannelSigner for TestChannelSigner {
 				state.last_holder_commitment
 			);
 		}
+
+		self.inner.validate_holder_commitment(
+			channel_parameters,
+			holder_tx,
+			outbound_htlc_preimages,
+			secp_ctx,
+		)?;
+
 		state.last_holder_commitment = idx;
 		Ok(())
 	}

### lightning/src/util/test_utils.rs
@@ -2080,6 +2080,7 @@ impl TestSignerFactory for DefaultSignerFactory {
 			now.subsec_nanos(),
 			if let Some(provided_seed) = phantom_seed { provided_seed } else { seed },
 			v2_remote_key_derivation,
+			Arc::new(TestLogger::new()),
 		);
 		let dphantom = DynPhantomKeysInterface::new(phantom);
 		let backing = Box::new(dphantom) as Box<dyn DynKeysInterfaceTrait<EcdsaSigner = DynSigner>>;