Separate ChannelManager constructor methods for public vs private nodes

Public nodes are now required to provide a router, since they are expected to
forward payments.

In upcoming work, we will refuse to broadcast channel updates for private
nodes.

Also switch ValidatedBlockHeader::to_best_block from spaces to tabs

Author: valentinewallace

fuzz/src/chanmon_consistency.rs

@@ -394,7 +394,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 				network,
 				best_block: BestBlock::from_genesis(network),
 			};
-			(ChannelManager::new($fee_estimator.clone(), monitor.clone(), broadcast.clone(), Some(router.clone()), Arc::clone(&logger), keys_manager.clone(), config, params),
+			(ChannelManager::new_public($fee_estimator.clone(), monitor.clone(), broadcast.clone(), router.clone(), Arc::clone(&logger), keys_manager.clone(), config, params),
 			monitor, keys_manager)
 		} }
 	}

fuzz/src/full_stack.rs

@@ -418,7 +418,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 		network,
 		best_block: BestBlock::from_genesis(network),
 	};
-	let channelmanager = Arc::new(ChannelManager::new(fee_est.clone(), monitor.clone(), broadcast.clone(), Some(router), Arc::clone(&logger), keys_manager.clone(), config, params));
+	let channelmanager = Arc::new(ChannelManager::new_public(fee_est.clone(), monitor.clone(), broadcast.clone(), router, Arc::clone(&logger), keys_manager.clone(), config, params));
 	// Adding new calls to `KeysInterface::get_secure_random_bytes` during startup can change all the
 	// keys subsequently generated in this test. Rather than regenerating all the messages manually,
 	// it's easier to just increment the counter here so the keys don't change.

lightning-background-processor/src/lib.rs

@@ -748,7 +748,7 @@ mod tests {
 			let chain_monitor = Arc::new(chainmonitor::ChainMonitor::new(Some(chain_source.clone()), tx_broadcaster.clone(), logger.clone(), fee_estimator.clone(), persister.clone()));
 			let best_block = BestBlock::from_genesis(network);
 			let params = ChainParameters { network, best_block };
-			let manager = Arc::new(ChannelManager::new(fee_estimator.clone(), chain_monitor.clone(), tx_broadcaster.clone(), Some(router.clone()), logger.clone(), keys_manager.clone(), UserConfig::default(), params));
+			let manager = Arc::new(ChannelManager::new_public(fee_estimator.clone(), chain_monitor.clone(), tx_broadcaster.clone(), router.clone(), logger.clone(), keys_manager.clone(), UserConfig::default(), params));
 			let network_graph = Arc::new(NetworkGraph::new(genesis_block.header.block_hash(), logger.clone()));
 			let p2p_gossip_sync = Arc::new(P2PGossipSync::new(network_graph.clone(), Some(chain_source.clone()), logger.clone()));
 			let rapid_gossip_sync = Arc::new(RapidGossipSync::new(network_graph.clone()));

lightning-block-sync/src/poll.rs

@@ -148,18 +148,19 @@ impl ValidatedBlockHeader {
 		Ok(())
 	}
 
-    /// Returns the [`BestBlock`] corresponding to this validated block header, which can be passed
-    /// into [`ChannelManager::new`] as part of its [`ChainParameters`]. Useful for ensuring that
-    /// the [`SpvClient`] and [`ChannelManager`] are initialized to the same block during a fresh
-    /// start.
-    ///
-    /// [`SpvClient`]: crate::SpvClient
-    /// [`ChainParameters`]: lightning::ln::channelmanager::ChainParameters
-    /// [`ChannelManager`]: lightning::ln::channelmanager::ChannelManager
-    /// [`ChannelManager::new`]: lightning::ln::channelmanager::ChannelManager::new
-    pub fn to_best_block(&self) -> BestBlock {
-        BestBlock::new(self.block_hash, self.inner.height)
-    }
+	/// Returns the [`BestBlock`] corresponding to this validated block header, which can be passed
+	/// into [`ChannelManager::new_public`] and [`ChannelManager::new_private`] as part of their
+	/// [`ChainParameters`]. Useful for ensuring that the [`SpvClient`] and [`ChannelManager`] are
+	/// initialized to the same block during a fresh start.
+	///
+	/// [`SpvClient`]: crate::SpvClient
+	/// [`ChainParameters`]: lightning::ln::channelmanager::ChainParameters
+	/// [`ChannelManager`]: lightning::ln::channelmanager::ChannelManager
+	/// [`ChannelManager::new_public`]: lightning::ln::channelmanager::ChannelManager::new_public
+	/// [`ChannelManager::new_private`]: lightning::ln::channelmanager::ChannelManager::new_private
+	pub fn to_best_block(&self) -> BestBlock {
+		BestBlock::new(self.block_hash, self.inner.height)
+	}
 }
 
 /// A block with validated data against its transaction list and corresponding block hash.

lightning/src/ln/channelmanager.rs

@@ -119,6 +119,27 @@ pub trait Router {
 	fn notify_payment_path_successful(&self, path: &[&RouteHop]);
 }
 
+// Used in private nodes that do not forward payments, since ChannelManager's router is only used in
+// trampoline forwards.
+struct IgnoringRouter {}
+impl Deref for IgnoringRouter {
+	type Target = IgnoringRouter;
+	fn deref(&self) -> &Self { self }
+}
+impl Router for IgnoringRouter {
+	fn find_route(
+		&self, _payer: &PublicKey, _route_params: &RouteParameters,
+		_first_hops: Option<&[&ChannelDetails]>, _inflight_htlcs: InFlightHtlcs
+	) -> Result<Route, LightningError> {
+		Err(msgs::LightningError {
+			err: String::from("Not implemented"),
+			action: msgs::ErrorAction::IgnoreError
+		})
+	}
+	fn notify_payment_path_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
+	fn notify_payment_path_successful(&self, _path: &[&RouteHop]) {}
+}
+
 // We hold various information about HTLC relay in the HTLC objects in Channel itself:
 //
 // Upon receipt of an HTLC from a peer, we'll give it a PendingHTLCStatus indicating if it should
@@ -1643,6 +1664,20 @@ macro_rules! post_handle_chan_restoration {
 	} }
 }
 
+impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F, IgnoringRouter, L>
+	where M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
+	      T::Target: BroadcasterInterface,
+	      K::Target: KeysInterface,
+	      F::Target: FeeEstimator,
+	      L::Target: Logger,
+{
+	/// Similar to [`ChannelManager::new_public`], but used in private nodes that do not support
+	/// forwarding payments. None of it channels will be announced to the network.
+	pub fn new_private(fee_est: F, chain_monitor: M, tx_broadcaster: T, logger: L, keys_manager: K, config: UserConfig, params: ChainParameters) -> Self {
+		Self::new(fee_est, chain_monitor, tx_broadcaster, IgnoringRouter {}, logger, keys_manager, config, params, false)
+	}
+}
+
 impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<M, T, K, F, R, L>
 	where M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
 	      T::Target: BroadcasterInterface,
@@ -1652,6 +1687,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 	      L::Target: Logger,
 {
 	/// Constructs a new ChannelManager to hold several channels and route between them.
+	/// For use in a public lightning node that supports forwarding payments.
 	///
 	/// This is the main "logic hub" for all channel-related actions, and implements
 	/// ChannelMessageHandler.
@@ -1661,7 +1697,11 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 	/// Users need to notify the new ChannelManager when a new block is connected or
 	/// disconnected using its `block_connected` and `block_disconnected` methods, starting
 	/// from after `params.latest_hash`.
-	pub fn new(fee_est: F, chain_monitor: M, tx_broadcaster: T, router: Option<R>, logger: L, keys_manager: K, config: UserConfig, params: ChainParameters) -> Self {
+	pub fn new_public(fee_est: F, chain_monitor: M, tx_broadcaster: T, router: R, logger: L, keys_manager: K, config: UserConfig, params: ChainParameters) -> Self {
+		Self::new(fee_est, chain_monitor, tx_broadcaster, router, logger, keys_manager, config, params, true)
+	}
+
+	fn new(fee_est: F, chain_monitor: M, tx_broadcaster: T, router: R, logger: L, keys_manager: K, config: UserConfig, params: ChainParameters, is_public: bool) -> Self {
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
 		let inbound_pmt_key_material = keys_manager.get_inbound_payment_key_material();
@@ -1672,7 +1712,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 			fee_estimator: LowerBoundedFeeEstimator::new(fee_est),
 			chain_monitor,
 			tx_broadcaster,
-			router,
+			router: if is_public { Some(router) } else { None },
 
 			best_block: RwLock::new(params.best_block),
 
@@ -7996,7 +8036,7 @@ pub mod bench {
 		let chain_monitor_a = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_a);
 		let seed_a = [1u8; 32];
 		let keys_manager_a = KeysManager::new(&seed_a, 42, 42);
-		let node_a = ChannelManager::new(&fee_estimator, &chain_monitor_a, &tx_broadcaster, Some(&router), &logger_a, &keys_manager_a, config.clone(), ChainParameters {
+		let node_a = ChannelManager::new_public(&fee_estimator, &chain_monitor_a, &tx_broadcaster, &router, &logger_a, &keys_manager_a, config.clone(), ChainParameters {
 			network,
 			best_block: BestBlock::from_genesis(network),
 		});
@@ -8006,7 +8046,7 @@ pub mod bench {
 		let chain_monitor_b = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_b);
 		let seed_b = [2u8; 32];
 		let keys_manager_b = KeysManager::new(&seed_b, 42, 42);
-		let node_b = ChannelManager::new(&fee_estimator, &chain_monitor_b, &tx_broadcaster, Some(&router), &logger_b, &keys_manager_b, config.clone(), ChainParameters {
+		let node_b = ChannelManager::new_public(&fee_estimator, &chain_monitor_b, &tx_broadcaster, &router, &logger_b, &keys_manager_b, config.clone(), ChainParameters {
 			network,
 			best_block: BestBlock::from_genesis(network),
 		});

lightning/src/ln/functional_test_utils.rs

@@ -2082,7 +2082,7 @@ pub fn create_node_chanmgrs<'a, 'b>(node_count: usize, cfgs: &'a Vec<NodeCfg<'b>
 			network,
 			best_block: BestBlock::from_genesis(network),
 		};
-		let node = ChannelManager::new(cfgs[i].fee_estimator, &cfgs[i].chain_monitor, cfgs[i].tx_broadcaster, Some(&cfgs[i].router), cfgs[i].logger, cfgs[i].keys_manager,
+		let node = ChannelManager::new_public(cfgs[i].fee_estimator, &cfgs[i].chain_monitor, cfgs[i].tx_broadcaster, &cfgs[i].router, cfgs[i].logger, cfgs[i].keys_manager,
 			if node_config[i].is_some() { node_config[i].clone().unwrap() } else { test_default_channel_config() }, params);
 		chanmgrs.push(node);
 	}