Drop the async-setting of the P2PGossipSync utxo_verifier

Now that we do not rely on circular references for `P2PGossipSync`
validation, we no longer need the hacky
`P2PGossipSync::add_utxo_lookup` method to add the gossip
validator after building the `P2PGossipSync` first. Thus, we
remove it here, updating some tests that relied on it.

Author: TheBlueMatt

lightning/src/routing/gossip.rs

@@ -328,7 +328,10 @@ where
 	L::Target: Logger,
 {
 	network_graph: G,
-	utxo_lookup: RwLock<Option<U>>,
+	#[cfg(any(feature = "_test_utils", test))]
+	pub(super) utxo_lookup: Option<U>,
+	#[cfg(not(any(feature = "_test_utils", test)))]
+	utxo_lookup: Option<U>,
 	full_syncs_requested: AtomicUsize,
 	pending_events: Mutex<Vec<MessageSendEvent>>,
 	logger: L,
@@ -341,25 +344,19 @@ where
 {
 	/// Creates a new tracker of the actual state of the network of channels and nodes,
 	/// assuming an existing [`NetworkGraph`].
+	///
 	/// UTXO lookup is used to make sure announced channels exist on-chain, channel data is
 	/// correct, and the announcement is signed with channel owners' keys.
 	pub fn new(network_graph: G, utxo_lookup: Option<U>, logger: L) -> Self {
 		P2PGossipSync {
 			network_graph,
 			full_syncs_requested: AtomicUsize::new(0),
-			utxo_lookup: RwLock::new(utxo_lookup),
+			utxo_lookup,
 			pending_events: Mutex::new(vec![]),
 			logger,
 		}
 	}
 
-	/// Adds a provider used to check new announcements. Does not affect
-	/// existing announcements unless they are updated.
-	/// Add, update or remove the provider would replace the current one.
-	pub fn add_utxo_lookup(&self, utxo_lookup: Option<U>) {
-		*self.utxo_lookup.write().unwrap() = utxo_lookup;
-	}
-
 	/// Gets a reference to the underlying [`NetworkGraph`] which was provided in
 	/// [`P2PGossipSync::new`].
 	///
@@ -564,8 +561,7 @@ where
 	fn handle_channel_announcement(
 		&self, _their_node_id: Option<PublicKey>, msg: &msgs::ChannelAnnouncement,
 	) -> Result<bool, LightningError> {
-		self.network_graph
-			.update_channel_from_announcement(msg, &*self.utxo_lookup.read().unwrap())?;
+		self.network_graph.update_channel_from_announcement(msg, &self.utxo_lookup)?;
 		Ok(msg.contents.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY)
 	}
 

lightning/src/routing/router.rs

@@ -3943,10 +3943,7 @@ mod tests {
 		ChannelUsage, FixedPenaltyScorer, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
 		ProbabilisticScoringFeeParameters, ScoreLookUp,
 	};
-	use crate::routing::test_utils::{
-		add_channel, add_or_update_node, build_graph, build_line_graph, get_nodes,
-		id_to_feature_flags, update_channel,
-	};
+	use crate::routing::test_utils::*;
 	use crate::routing::utxo::UtxoResult;
 	use crate::types::features::{BlindedHopFeatures, ChannelFeatures, InitFeatures, NodeFeatures};
 	use crate::util::config::UserConfig;
@@ -5368,7 +5365,7 @@ mod tests {
 	fn available_amount_while_routing_test() {
 		// Tests whether we choose the correct available channel amount while routing.
 
-		let (secp_ctx, network_graph, gossip_sync, chain_monitor, logger) = build_graph();
+		let (secp_ctx, network_graph, gossip_sync, chain_monitor, logger) = build_graph_with_gossip_validation();
 		let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 		let scorer = ln_test_utils::TestScorer::new();
 		let random_seed_bytes = [42; 32];
@@ -5588,11 +5585,10 @@ mod tests {
 		.push_opcode(opcodes::all::OP_PUSHNUM_2)
 		.push_opcode(opcodes::all::OP_CHECKMULTISIG).into_script().to_p2wsh();
 
+
 		*chain_monitor.utxo_ret.lock().unwrap() =
 			UtxoResult::Sync(Ok(TxOut { value: Amount::from_sat(15), script_pubkey: good_script.clone() }));
-		gossip_sync.add_utxo_lookup(Some(chain_monitor));
-
-		add_channel(&gossip_sync, &secp_ctx, &privkeys[0], &privkeys[2], ChannelFeatures::from_le_bytes(id_to_feature_flags(3)), 333);
+		add_channel_skipping_utxo_update(&gossip_sync, &secp_ctx, &privkeys[0], &privkeys[2], ChannelFeatures::from_le_bytes(id_to_feature_flags(3)), 333);
 		update_channel(&gossip_sync, &secp_ctx, &privkeys[0], UnsignedChannelUpdate {
 			chain_hash: ChainHash::using_genesis_block(Network::Testnet),
 			short_channel_id: 333,

lightning/src/routing/test_utils.rs

@@ -10,7 +10,9 @@
 // licenses.
 
 use crate::routing::gossip::{NetworkGraph, NodeAlias, P2PGossipSync};
+use crate::routing::utxo::UtxoResult;
 use crate::types::features::{ChannelFeatures, NodeFeatures};
+use crate::ln::chan_utils::make_funding_redeemscript;
 use crate::ln::msgs::{ChannelAnnouncement, ChannelUpdate, MAX_VALUE_MSAT, NodeAnnouncement, RoutingMessageHandler, SocketAddress, UnsignedChannelAnnouncement, UnsignedChannelUpdate, UnsignedNodeAnnouncement};
 use crate::util::test_utils;
 use crate::util::ser::Writeable;
@@ -22,6 +24,7 @@ use bitcoin::hex::FromHex;
 use bitcoin::network::Network;
 use bitcoin::secp256k1::{PublicKey,SecretKey};
 use bitcoin::secp256k1::{Secp256k1, All};
+use bitcoin::{Amount, TxOut};
 
 #[allow(unused)]
 use crate::prelude::*;
@@ -58,19 +61,34 @@ pub(crate) fn channel_announcement(
 }
 
 // Using the same keys for LN and BTC ids
-pub(crate) fn add_channel(
+pub(crate) fn add_channel_skipping_utxo_update(
 	gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
-	secp_ctx: &Secp256k1<All>, node_1_privkey: &SecretKey, node_2_privkey: &SecretKey, features: ChannelFeatures, short_channel_id: u64
+	secp_ctx: &Secp256k1<All>, node_1_privkey: &SecretKey, node_2_privkey: &SecretKey, features: ChannelFeatures, short_channel_id: u64,
 ) {
 	let valid_announcement =
 		channel_announcement(node_1_privkey, node_2_privkey, features, short_channel_id, secp_ctx);
-	let node_1_pubkey = PublicKey::from_secret_key(&secp_ctx, node_1_privkey);
+
+	let node_1_pubkey = PublicKey::from_secret_key(&secp_ctx, &node_1_privkey);
 	match gossip_sync.handle_channel_announcement(Some(node_1_pubkey), &valid_announcement) {
 		Ok(res) => assert!(res),
-		_ => panic!()
+		Err(e) => panic!("{:?}", e),
 	};
 }
 
+pub(crate) fn add_channel(
+	gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
+	secp_ctx: &Secp256k1<All>, node_1_privkey: &SecretKey, node_2_privkey: &SecretKey, features: ChannelFeatures, short_channel_id: u64,
+) {
+	gossip_sync.utxo_lookup.as_ref().map(|checker| {
+		let node_1_pubkey = PublicKey::from_secret_key(&secp_ctx, &node_1_privkey);
+		let node_2_pubkey = PublicKey::from_secret_key(&secp_ctx, &node_2_privkey);
+		let script_pubkey = make_funding_redeemscript(&node_1_pubkey, &node_2_pubkey).to_p2wsh();
+		*checker.utxo_ret.lock().unwrap() =
+			UtxoResult::Sync(Ok(TxOut { value: Amount::from_sat(21_000_000_0000_0000), script_pubkey }));
+	});
+	add_channel_skipping_utxo_update(gossip_sync, secp_ctx, node_1_privkey, node_2_privkey, features, short_channel_id);
+}
+
 pub(crate) fn add_or_update_node(
 	gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
 	secp_ctx: &Secp256k1<All>, node_privkey: &SecretKey, features: NodeFeatures, timestamp: u32
@@ -197,18 +215,43 @@ pub(super) fn build_line_graph() -> (
 	(secp_ctx, network_graph, gossip_sync, chain_monitor, logger)
 }
 
+pub(super) fn build_graph_with_gossip_validation() -> (
+	Secp256k1<All>,
+	sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
+	P2PGossipSync<sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>>,
+	sync::Arc<test_utils::TestChainSource>,
+	sync::Arc<test_utils::TestLogger>,
+) {
+	do_build_graph(true)
+}
+
 pub(super) fn build_graph() -> (
 	Secp256k1<All>,
 	sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
 	P2PGossipSync<sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>>,
 	sync::Arc<test_utils::TestChainSource>,
 	sync::Arc<test_utils::TestLogger>,
+) {
+	do_build_graph(false)
+}
+
+fn do_build_graph(with_validation: bool) -> (
+	Secp256k1<All>,
+	sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
+	P2PGossipSync<sync::Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, sync::Arc<test_utils::TestChainSource>, sync::Arc<test_utils::TestLogger>>,
+	sync::Arc<test_utils::TestChainSource>,
+	sync::Arc<test_utils::TestLogger>,
 ) {
 	let secp_ctx = Secp256k1::new();
 	let logger = Arc::new(test_utils::TestLogger::new());
 	let chain_monitor = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
 	let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, Arc::clone(&logger)));
-	let gossip_sync = P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger));
+	let checker = if with_validation {
+		Some(Arc::clone(&chain_monitor))
+	} else {
+		None
+	};
+	let gossip_sync = P2PGossipSync::new(Arc::clone(&network_graph), checker, Arc::clone(&logger));
 	// Build network from our_id to node6:
 	//
 	//        -1(1)2-  node0  -1(3)2-