Linting: Disallow unwraps, replace with expect or error propagation

.github/workflows/rust.yml

@@ -90,6 +90,21 @@ jobs:
         run: |
           RUSTFLAGS="--cfg no_download --cfg cycle_tests" cargo test --features uniffi
 
+  linting:
+    name: Linting
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout source code
+        uses: actions/checkout@v6
+      - name: Install Rust and clippy
+        run: |
+          curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
+          rustup component add clippy
+      - name: Ban `unwrap` in library code
+        run: |
+          cargo clippy --lib --verbose --color always -- -A warnings -D clippy::unwrap_used -A clippy::tabs_in_doc_comments
+          cargo clippy --lib --features uniffi --verbose --color always -- -A warnings -D clippy::unwrap_used -A clippy::tabs_in_doc_comments
+
   doc:
     name: Documentation
     runs-on: ubuntu-latest

build.rs

@@ -7,5 +7,6 @@
 
 fn main() {
 	#[cfg(feature = "uniffi")]
-	uniffi::generate_scaffolding("bindings/ldk_node.udl").unwrap();
+	uniffi::generate_scaffolding("bindings/ldk_node.udl")
+		.expect("the checked-in UniFFI UDL should always generate scaffolding");
 }

src/balance.rs

@@ -232,7 +232,9 @@ impl LightningBalance {
 				inbound_htlc_rounded_msat,
 			} => {
 				// unwrap safety: confirmed_balance_candidate_index is guaranteed to index into balance_candidates
-				let balance = balance_candidates.get(confirmed_balance_candidate_index).unwrap();
+				let balance = balance_candidates
+					.get(confirmed_balance_candidate_index)
+					.expect("LDK should provide a valid confirmed balance candidate index");
 
 				Self::ClaimableOnChannelClose {
 					channel_id,

src/builder.rs

@@ -189,6 +189,8 @@ pub enum BuildError {
 	WalletSetupFailed,
 	/// We failed to setup the logger.
 	LoggerSetupFailed,
+	/// We failed to setup the configured chain source.
+	ChainSourceSetupFailed,
 	/// The given network does not match the node's previously configured network.
 	NetworkMismatch,
 	/// The role of the node in an asynchronous payments context is not compatible with the current configuration.
@@ -216,6 +218,7 @@ impl fmt::Display for BuildError {
 			Self::KVStoreSetupFailed => write!(f, "Failed to setup KVStore."),
 			Self::WalletSetupFailed => write!(f, "Failed to setup onchain wallet."),
 			Self::LoggerSetupFailed => write!(f, "Failed to setup the logger."),
+			Self::ChainSourceSetupFailed => write!(f, "Failed to setup the chain source."),
 			Self::InvalidNodeAlias => write!(f, "Given node alias is invalid."),
 			Self::NetworkMismatch => {
 				write!(f, "Given network does not match the node's previously configured network.")
@@ -861,7 +864,7 @@ impl ArcedNodeBuilder {
 	pub fn set_chain_source_esplora(
 		&self, server_url: String, sync_config: Option<EsploraSyncConfig>,
 	) {
-		self.inner.write().unwrap().set_chain_source_esplora(server_url, sync_config);
+		self.inner.write().expect("lock").set_chain_source_esplora(server_url, sync_config);
 	}
 
 	/// Configures the [`Node`] instance to source its chain data from the given Esplora server.
@@ -875,7 +878,7 @@ impl ArcedNodeBuilder {
 		&self, server_url: String, headers: HashMap<String, String>,
 		sync_config: Option<EsploraSyncConfig>,
 	) {
-		self.inner.write().unwrap().set_chain_source_esplora_with_headers(
+		self.inner.write().expect("lock").set_chain_source_esplora_with_headers(
 			server_url,
 			headers,
 			sync_config,
@@ -889,7 +892,7 @@ impl ArcedNodeBuilder {
 	pub fn set_chain_source_electrum(
 		&self, server_url: String, sync_config: Option<ElectrumSyncConfig>,
 	) {
-		self.inner.write().unwrap().set_chain_source_electrum(server_url, sync_config);
+		self.inner.write().expect("lock").set_chain_source_electrum(server_url, sync_config);
 	}
 
 	/// Configures the [`Node`] instance to connect to a Bitcoin Core node via RPC.
@@ -903,7 +906,7 @@ impl ArcedNodeBuilder {
 	pub fn set_chain_source_bitcoind_rpc(
 		&self, rpc_host: String, rpc_port: u16, rpc_user: String, rpc_password: String,
 	) {
-		self.inner.write().unwrap().set_chain_source_bitcoind_rpc(
+		self.inner.write().expect("lock").set_chain_source_bitcoind_rpc(
 			rpc_host,
 			rpc_port,
 			rpc_user,
@@ -924,7 +927,7 @@ impl ArcedNodeBuilder {
 		&self, rest_host: String, rest_port: u16, rpc_host: String, rpc_port: u16,
 		rpc_user: String, rpc_password: String,
 	) {
-		self.inner.write().unwrap().set_chain_source_bitcoind_rest(
+		self.inner.write().expect("lock").set_chain_source_bitcoind_rest(
 			rest_host,
 			rest_port,
 			rpc_host,
@@ -937,20 +940,20 @@ impl ArcedNodeBuilder {
 	/// Configures the [`Node`] instance to source its gossip data from the Lightning peer-to-peer
 	/// network.
 	pub fn set_gossip_source_p2p(&self) {
-		self.inner.write().unwrap().set_gossip_source_p2p();
+		self.inner.write().expect("lock").set_gossip_source_p2p();
 	}
 
 	/// Configures the [`Node`] instance to source its gossip data from the given RapidGossipSync
 	/// server.
 	pub fn set_gossip_source_rgs(&self, rgs_server_url: String) {
-		self.inner.write().unwrap().set_gossip_source_rgs(rgs_server_url);
+		self.inner.write().expect("lock").set_gossip_source_rgs(rgs_server_url);
 	}
 
 	/// Configures the [`Node`] instance to source its external scores from the given URL.
 	///
 	/// The external scores are merged into the local scoring system to improve routing.
 	pub fn set_pathfinding_scores_source(&self, url: String) {
-		self.inner.write().unwrap().set_pathfinding_scores_source(url);
+		self.inner.write().expect("lock").set_pathfinding_scores_source(url);
 	}
 
 	/// Configures the [`Node`] instance to source inbound liquidity from the given
@@ -964,7 +967,7 @@ impl ArcedNodeBuilder {
 	pub fn set_liquidity_source_lsps1(
 		&self, node_id: PublicKey, address: SocketAddress, token: Option<String>,
 	) {
-		self.inner.write().unwrap().set_liquidity_source_lsps1(node_id, address, token);
+		self.inner.write().expect("lock").set_liquidity_source_lsps1(node_id, address, token);
 	}
 
 	/// Configures the [`Node`] instance to source just-in-time inbound liquidity from the given
@@ -978,7 +981,7 @@ impl ArcedNodeBuilder {
 	pub fn set_liquidity_source_lsps2(
 		&self, node_id: PublicKey, address: SocketAddress, token: Option<String>,
 	) {
-		self.inner.write().unwrap().set_liquidity_source_lsps2(node_id, address, token);
+		self.inner.write().expect("lock").set_liquidity_source_lsps2(node_id, address, token);
 	}
 
 	/// Configures the [`Node`] instance to provide an [LSPS2] service, issuing just-in-time
@@ -988,12 +991,12 @@ impl ArcedNodeBuilder {
 	///
 	/// [LSPS2]: https://github.com/BitcoinAndLightningLayerSpecs/lsp/blob/main/LSPS2/README.md
 	pub fn set_liquidity_provider_lsps2(&self, service_config: LSPS2ServiceConfig) {
-		self.inner.write().unwrap().set_liquidity_provider_lsps2(service_config);
+		self.inner.write().expect("lock").set_liquidity_provider_lsps2(service_config);
 	}
 
 	/// Sets the used storage directory path.
 	pub fn set_storage_dir_path(&self, storage_dir_path: String) {
-		self.inner.write().unwrap().set_storage_dir_path(storage_dir_path);
+		self.inner.write().expect("lock").set_storage_dir_path(storage_dir_path);
 	}
 
 	/// Configures the [`Node`] instance to write logs to the filesystem.
@@ -1012,29 +1015,29 @@ impl ArcedNodeBuilder {
 	pub fn set_filesystem_logger(
 		&self, log_file_path: Option<String>, log_level: Option<LogLevel>,
 	) {
-		self.inner.write().unwrap().set_filesystem_logger(log_file_path, log_level);
+		self.inner.write().expect("lock").set_filesystem_logger(log_file_path, log_level);
 	}
 
 	/// Configures the [`Node`] instance to write logs to the [`log`](https://crates.io/crates/log) facade.
 	pub fn set_log_facade_logger(&self) {
-		self.inner.write().unwrap().set_log_facade_logger();
+		self.inner.write().expect("lock").set_log_facade_logger();
 	}
 
 	/// Configures the [`Node`] instance to write logs to the provided custom [`LogWriter`].
 	pub fn set_custom_logger(&self, log_writer: Arc<dyn LogWriter>) {
-		self.inner.write().unwrap().set_custom_logger(log_writer);
+		self.inner.write().expect("lock").set_custom_logger(log_writer);
 	}
 
 	/// Sets the Bitcoin network used.
 	pub fn set_network(&self, network: Network) {
-		self.inner.write().unwrap().set_network(network);
+		self.inner.write().expect("lock").set_network(network);
 	}
 
 	/// Sets the IP address and TCP port on which [`Node`] will listen for incoming network connections.
 	pub fn set_listening_addresses(
 		&self, listening_addresses: Vec<SocketAddress>,
 	) -> Result<(), BuildError> {
-		self.inner.write().unwrap().set_listening_addresses(listening_addresses).map(|_| ())
+		self.inner.write().expect("lock").set_listening_addresses(listening_addresses).map(|_| ())
 	}
 
 	/// Sets the IP address and TCP port which [`Node`] will announce to the gossip network that it accepts connections on.
@@ -1045,7 +1048,11 @@ impl ArcedNodeBuilder {
 	pub fn set_announcement_addresses(
 		&self, announcement_addresses: Vec<SocketAddress>,
 	) -> Result<(), BuildError> {
-		self.inner.write().unwrap().set_announcement_addresses(announcement_addresses).map(|_| ())
+		self.inner
+			.write()
+			.expect("lock")
+			.set_announcement_addresses(announcement_addresses)
+			.map(|_| ())
 	}
 
 	/// Configures the [`Node`] instance to use a Tor SOCKS proxy for outbound connections to peers with OnionV3 addresses.
@@ -1054,22 +1061,22 @@ impl ArcedNodeBuilder {
 	///
 	/// **Note**: If unset, connecting to peer OnionV3 addresses will fail.
 	pub fn set_tor_config(&self, tor_config: TorConfig) -> Result<(), BuildError> {
-		self.inner.write().unwrap().set_tor_config(tor_config).map(|_| ())
+		self.inner.write().expect("lock").set_tor_config(tor_config).map(|_| ())
 	}
 
 	/// Sets the node alias that will be used when broadcasting announcements to the gossip
 	/// network.
 	///
 	/// The provided alias must be a valid UTF-8 string and no longer than 32 bytes in total.
 	pub fn set_node_alias(&self, node_alias: String) -> Result<(), BuildError> {
-		self.inner.write().unwrap().set_node_alias(node_alias).map(|_| ())
+		self.inner.write().expect("lock").set_node_alias(node_alias).map(|_| ())
 	}
 
 	/// Sets the role of the node in an asynchronous payments context.
 	pub fn set_async_payments_role(
 		&self, role: Option<AsyncPaymentsRole>,
 	) -> Result<(), BuildError> {
-		self.inner.write().unwrap().set_async_payments_role(role).map(|_| ())
+		self.inner.write().expect("lock").set_async_payments_role(role).map(|_| ())
 	}
 
 	/// Configures the [`Node`] to resync chain data from genesis on first startup, recovering any
@@ -1078,21 +1085,21 @@ impl ArcedNodeBuilder {
 	/// This should only be set on first startup when importing an older wallet from a previously
 	/// used [`NodeEntropy`].
 	pub fn set_wallet_recovery_mode(&self) {
-		self.inner.write().unwrap().set_wallet_recovery_mode();
+		self.inner.write().expect("lock").set_wallet_recovery_mode();
 	}
 
 	/// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options
 	/// previously configured.
 	pub fn build(&self, node_entropy: Arc<NodeEntropy>) -> Result<Arc<Node>, BuildError> {
-		self.inner.read().unwrap().build(*node_entropy).map(Arc::new)
+		self.inner.read().expect("lock").build(*node_entropy).map(Arc::new)
 	}
 
 	/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
 	/// previously configured.
 	pub fn build_with_fs_store(
 		&self, node_entropy: Arc<NodeEntropy>,
 	) -> Result<Arc<Node>, BuildError> {
-		self.inner.read().unwrap().build_with_fs_store(*node_entropy).map(Arc::new)
+		self.inner.read().expect("lock").build_with_fs_store(*node_entropy).map(Arc::new)
 	}
 
 	/// Builds a [`Node`] instance with a [VSS] backend and according to the options
@@ -1118,7 +1125,7 @@ impl ArcedNodeBuilder {
 	) -> Result<Arc<Node>, BuildError> {
 		self.inner
 			.read()
-			.unwrap()
+			.expect("lock")
 			.build_with_vss_store(*node_entropy, vss_url, store_id, fixed_headers)
 			.map(Arc::new)
 	}
@@ -1151,7 +1158,7 @@ impl ArcedNodeBuilder {
 	) -> Result<Arc<Node>, BuildError> {
 		self.inner
 			.read()
-			.unwrap()
+			.expect("lock")
 			.build_with_vss_store_and_lnurl_auth(
 				*node_entropy,
 				vss_url,
@@ -1180,7 +1187,7 @@ impl ArcedNodeBuilder {
 	) -> Result<Arc<Node>, BuildError> {
 		self.inner
 			.read()
-			.unwrap()
+			.expect("lock")
 			.build_with_vss_store_and_fixed_headers(*node_entropy, vss_url, store_id, fixed_headers)
 			.map(Arc::new)
 	}
@@ -1203,7 +1210,7 @@ impl ArcedNodeBuilder {
 		let adapter = Arc::new(crate::ffi::VssHeaderProviderAdapter::new(header_provider));
 		self.inner
 			.read()
-			.unwrap()
+			.expect("lock")
 			.build_with_vss_store_and_header_provider(*node_entropy, vss_url, store_id, adapter)
 			.map(Arc::new)
 	}
@@ -1214,7 +1221,7 @@ impl ArcedNodeBuilder {
 	pub fn build_with_store<S: SyncAndAsyncKVStore + Send + Sync + 'static>(
 		&self, node_entropy: Arc<NodeEntropy>, kv_store: S,
 	) -> Result<Arc<Node>, BuildError> {
-		self.inner.read().unwrap().build_with_store(*node_entropy, kv_store).map(Arc::new)
+		self.inner.read().expect("lock").build_with_store(*node_entropy, kv_store).map(Arc::new)
 	}
 }
 
@@ -1310,6 +1317,7 @@ fn build_with_store_internal(
 				Arc::clone(&logger),
 				Arc::clone(&node_metrics),
 			)
+			.map_err(|()| BuildError::ChainSourceSetupFailed)?
 		},
 		Some(ChainDataSourceConfig::Electrum { server_url, sync_config }) => {
 			let sync_config = sync_config.unwrap_or(ElectrumSyncConfig::default());
@@ -1379,6 +1387,7 @@ fn build_with_store_internal(
 				Arc::clone(&logger),
 				Arc::clone(&node_metrics),
 			)
+			.map_err(|()| BuildError::ChainSourceSetupFailed)?
 		},
 	};
 	let chain_source = Arc::new(chain_source);
@@ -1610,7 +1619,7 @@ fn build_with_store_internal(
 	// Restore external pathfinding scores from cache if possible.
 	match external_scores_res {
 		Ok(external_scores) => {
-			scorer.lock().unwrap().merge(external_scores, cur_time);
+			scorer.lock().expect("lock").merge(external_scores, cur_time);
 			log_trace!(logger, "External scores from cache merged successfully");
 		},
 		Err(e) => {
@@ -1763,7 +1772,7 @@ fn build_with_store_internal(
 
 			// Reset the RGS sync timestamp in case we somehow switch gossip sources
 			{
-				let mut locked_node_metrics = node_metrics.write().unwrap();
+				let mut locked_node_metrics = node_metrics.write().expect("lock");
 				locked_node_metrics.latest_rgs_snapshot_timestamp = None;
 				write_node_metrics(&*locked_node_metrics, &*kv_store, Arc::clone(&logger))
 					.map_err(|e| {
@@ -1775,7 +1784,7 @@ fn build_with_store_internal(
 		},
 		GossipSourceConfig::RapidGossipSync(rgs_server) => {
 			let latest_sync_timestamp =
-				node_metrics.read().unwrap().latest_rgs_snapshot_timestamp.unwrap_or(0);
+				node_metrics.read().expect("lock").latest_rgs_snapshot_timestamp.unwrap_or(0);
 			Arc::new(GossipSource::new_rgs(
 				rgs_server.clone(),
 				latest_sync_timestamp,