Merge pull request #2788 from tnull/2023-12-enforce-no-warnings-ci

Deny warnings in CI

Author: TheBlueMatt

.github/workflows/build.yml

@@ -39,6 +39,9 @@ jobs:
         run: |
           sudo apt-get -y install shellcheck
           shellcheck ci/ci-tests.sh
+      - name: Set RUSTFLAGS to deny warnings
+        if: "matrix.toolchain == '1.63.0'"
+        run: echo "RUSTFLAGS=-D warnings" >> "$GITHUB_ENV"
       - name: Run CI script
         shell: bash # Default on Winblows is powershell
         run: CI_MINIMIZE_DISK_USAGE=1 ./ci/ci-tests.sh

ci/ci-tests.sh

@@ -94,14 +94,14 @@ if [[ "$HOST_PLATFORM" != *windows* ]]; then
 
 	DOWNLOAD_ELECTRS_AND_BITCOIND
 
-	RUSTFLAGS="--cfg no_download" cargo test --verbose --color always --features esplora-blocking
-	RUSTFLAGS="--cfg no_download" cargo check --verbose --color always --features esplora-blocking
-	RUSTFLAGS="--cfg no_download" cargo test --verbose --color always --features esplora-async
-	RUSTFLAGS="--cfg no_download" cargo check --verbose --color always --features esplora-async
-	RUSTFLAGS="--cfg no_download" cargo test --verbose --color always --features esplora-async-https
-	RUSTFLAGS="--cfg no_download" cargo check --verbose --color always --features esplora-async-https
-	RUSTFLAGS="--cfg no_download" cargo test --verbose --color always --features electrum
-	RUSTFLAGS="--cfg no_download" cargo check --verbose --color always --features electrum
+	RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo test --verbose --color always --features esplora-blocking
+	RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo check --verbose --color always --features esplora-blocking
+	RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo test --verbose --color always --features esplora-async
+	RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo check --verbose --color always --features esplora-async
+	RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo test --verbose --color always --features esplora-async-https
+	RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo check --verbose --color always --features esplora-async-https
+	RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo test --verbose --color always --features electrum
+	RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo check --verbose --color always --features electrum
 
 	popd
 fi
@@ -125,7 +125,7 @@ popd
 echo -e "\n\nBuilding with all Log-Limiting features"
 pushd lightning
 grep '^max_level_' Cargo.toml | awk '{ print $1 }'| while read -r FEATURE; do
-	cargo check --verbose --color always --features "$FEATURE"
+	RUSTFLAGS="$RUSTFLAGS -A unused_variables -A unused_macros -A unused_imports -A dead_code" cargo check --verbose --color always --features "$FEATURE"
 done
 popd
 
@@ -138,9 +138,9 @@ done
 
 for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do
 	# check if there is a conflict between no-std and the c_bindings cfg
-	RUSTFLAGS="--cfg=c_bindings" cargo test -p $DIR --verbose --color always --no-default-features --features=no-std
+	RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test -p $DIR --verbose --color always --no-default-features --features=no-std
 done
-RUSTFLAGS="--cfg=c_bindings" cargo test --verbose --color always
+RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test --verbose --color always
 
 # Note that outbound_commitment_test only runs in this mode because of hardcoded signature values
 pushd lightning
@@ -174,5 +174,5 @@ if [ -f "$(which arm-none-eabi-gcc)" ]; then
 fi
 
 echo -e "\n\nTest cfg-flag builds"
-RUSTFLAGS="$RUSTFLAGS --cfg=taproot" cargo test --verbose --color always -p lightning
-RUSTFLAGS="$RUSTFLAGS --cfg=async_signing" cargo test --verbose --color always -p lightning
+RUSTFLAGS="--cfg=taproot" cargo test --verbose --color always -p lightning
+RUSTFLAGS="--cfg=async_signing" cargo test --verbose --color always -p lightning

lightning-background-processor/src/lib.rs

@@ -27,7 +27,10 @@ use lightning::chain::chainmonitor::{ChainMonitor, Persist};
 use lightning::sign::{EntropySource, NodeSigner, SignerProvider};
 use lightning::events::{Event, PathFailure};
 #[cfg(feature = "std")]
-use lightning::events::{EventHandler, EventsProvider};
+use lightning::events::EventHandler;
+#[cfg(any(feature = "std", feature = "futures"))]
+use lightning::events::EventsProvider;
+
 use lightning::ln::channelmanager::ChannelManager;
 use lightning::ln::msgs::OnionMessageHandler;
 use lightning::ln::peer_handler::APeerManager;
@@ -727,8 +730,6 @@ async fn process_onion_message_handler_events_async<
 where
 	PM::Target: APeerManager + Send + Sync,
 {
-	use lightning::events::EventsProvider;
-
 	let events = core::cell::RefCell::new(Vec::new());
 	peer_manager.onion_message_handler().process_pending_events(&|e| events.borrow_mut().push(e));
 

lightning-block-sync/src/convert.rs

@@ -247,10 +247,12 @@ impl TryInto<BlockHash> for JsonResponse {
 /// The REST `getutxos` endpoint retuns a whole pile of data we don't care about and one bit we do
 /// - whether the `hit bitmap` field had any entries. Thus we condense the result down into only
 /// that.
+#[cfg(feature = "rest-client")]
 pub(crate) struct GetUtxosResponse {
 	pub(crate) hit_bitmap_nonempty: bool
 }
 
+#[cfg(feature = "rest-client")]
 impl TryInto<GetUtxosResponse> for JsonResponse {
 	type Error = std::io::Error;
 

lightning-invoice/src/lib.rs

@@ -77,6 +77,7 @@ mod de;
 mod ser;
 mod tb;
 
+#[allow(unused_imports)]
 mod prelude {
 	#[cfg(feature = "hashbrown")]
 	extern crate hashbrown;
@@ -92,10 +93,6 @@ mod prelude {
 
 use crate::prelude::*;
 
-/// Sync compat for std/no_std
-#[cfg(not(feature = "std"))]
-mod sync;
-
 /// Errors that indicate what is wrong with the invoice. They have some granularity for debug
 /// reasons, but should generally result in an "invalid BOLT11 invoice" message for the user.
 #[allow(missing_docs)]
@@ -2049,7 +2046,7 @@ mod test {
 		use lightning::routing::router::RouteHintHop;
 		use secp256k1::Secp256k1;
 		use secp256k1::{SecretKey, PublicKey};
-		use std::time::{UNIX_EPOCH, Duration};
+		use std::time::Duration;
 
 		let secp_ctx = Secp256k1::new();
 
@@ -2138,7 +2135,7 @@ mod test {
 		assert_eq!(invoice.currency(), Currency::BitcoinTestnet);
 		#[cfg(feature = "std")]
 		assert_eq!(
-			invoice.timestamp().duration_since(UNIX_EPOCH).unwrap().as_secs(),
+			invoice.timestamp().duration_since(SystemTime::UNIX_EPOCH).unwrap().as_secs(),
 			1234567
 		);
 		assert_eq!(invoice.payee_pub_key(), Some(&public_key));

lightning-invoice/src/payment.rs

@@ -85,14 +85,12 @@ mod tests {
 	use super::*;
 	use crate::{InvoiceBuilder, Currency};
 	use bitcoin::hashes::sha256::Hash as Sha256;
-	use lightning::events::Event;
-	use lightning::ln::channelmanager::{Retry, PaymentId};
-	use lightning::ln::msgs::ChannelMessageHandler;
 	use lightning::ln::PaymentSecret;
-	use lightning::ln::functional_test_utils::*;
 	use lightning::routing::router::Payee;
 	use secp256k1::{SecretKey, PublicKey, Secp256k1};
-	use std::time::{SystemTime, Duration};
+	use core::time::Duration;
+	#[cfg(feature = "std")]
+	use std::time::SystemTime;
 
 	fn duration_since_epoch() -> Duration {
 		#[cfg(feature = "std")]
@@ -171,6 +169,10 @@ mod tests {
 	#[test]
 	#[cfg(feature = "std")]
 	fn payment_metadata_end_to_end() {
+		use lightning::events::Event;
+		use lightning::ln::channelmanager::{Retry, PaymentId};
+		use lightning::ln::msgs::ChannelMessageHandler;
+		use lightning::ln::functional_test_utils::*;
 		// Test that a payment metadata read from an invoice passed to `pay_invoice` makes it all
 		// the way out through the `PaymentClaimable` event.
 		let chanmon_cfgs = create_chanmon_cfgs(2);

lightning-invoice/src/sync.rs

@@ -816,14 +816,15 @@ impl<'a, 'b, L: Deref> WithChannelDetails<'a, 'b, L> where L::Target: Logger {
 
 #[cfg(test)]
 mod test {
-	use core::cell::RefCell;
 	use core::time::Duration;
 	use crate::{Currency, Description, Bolt11InvoiceDescription, SignOrCreationError, CreationError};
 	use bitcoin::hashes::{Hash, sha256};
 	use bitcoin::hashes::sha256::Hash as Sha256;
 	use lightning::sign::PhantomKeysManager;
-	use lightning::events::{MessageSendEvent, MessageSendEventsProvider, Event, EventsProvider};
-	use lightning::ln::{PaymentPreimage, PaymentHash};
+	use lightning::events::{MessageSendEvent, MessageSendEventsProvider};
+	use lightning::ln::PaymentHash;
+	#[cfg(feature = "std")]
+	use lightning::ln::PaymentPreimage;
 	use lightning::ln::channelmanager::{PhantomRouteHints, MIN_FINAL_CLTV_EXPIRY_DELTA, PaymentId, RecipientOnionFields, Retry};
 	use lightning::ln::functional_test_utils::*;
 	use lightning::ln::msgs::ChannelMessageHandler;
@@ -1294,6 +1295,9 @@ mod test {
 
 	#[cfg(feature = "std")]
 	fn do_test_multi_node_receive(user_generated_pmt_hash: bool) {
+		use lightning::events::{Event, EventsProvider};
+		use core::cell::RefCell;
+
 		let mut chanmon_cfgs = create_chanmon_cfgs(3);
 		let seed_1 = [42u8; 32];
 		let seed_2 = [43u8; 32];

lightning-invoice/src/utils.rs

@@ -381,11 +381,6 @@ mod tests {
 	use lightning::util::persist::read_channel_monitors;
 	use std::fs;
 	use std::str::FromStr;
-	#[cfg(target_os = "windows")]
-	use {
-		lightning::get_event_msg,
-		lightning::ln::msgs::ChannelMessageHandler,
-	};
 
 	impl Drop for FilesystemStore {
 		fn drop(&mut self) {

lightning-persister/src/fs_store.rs

@@ -47,9 +47,7 @@ use crate::ln::channel::{Channel, ChannelPhase, ChannelContext, ChannelError, Ch
 use crate::ln::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
 #[cfg(any(feature = "_test_utils", test))]
 use crate::ln::features::Bolt11InvoiceFeatures;
-use crate::routing::gossip::NetworkGraph;
-use crate::routing::router::{BlindedTail, DefaultRouter, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteParameters, Router};
-use crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters};
+use crate::routing::router::{BlindedTail, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteParameters, Router};
 use crate::ln::onion_payment::{check_incoming_htlc_cltv, create_recv_pending_htlc_info, create_fwd_pending_htlc_info, decode_incoming_update_add_htlc_onion, InboundOnionErr, NextPacketDetails};
 use crate::ln::msgs;
 use crate::ln::onion_utils;
@@ -66,7 +64,7 @@ use crate::offers::offer::{DerivedMetadata, Offer, OfferBuilder};
 use crate::offers::parse::Bolt12SemanticError;
 use crate::offers::refund::{Refund, RefundBuilder};
 use crate::onion_message::{Destination, MessageRouter, OffersMessage, OffersMessageHandler, PendingOnionMessage, new_pending_onion_message};
-use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider};
+use crate::sign::{EntropySource, NodeSigner, Recipient, SignerProvider};
 use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
 use crate::util::config::{UserConfig, ChannelConfig, ChannelConfigUpdate};
 use crate::util::wakers::{Future, Notifier};
@@ -75,6 +73,13 @@ use crate::util::string::UntrustedString;
 use crate::util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter};
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::errors::APIError;
+#[cfg(not(c_bindings))]
+use {
+	crate::routing::router::DefaultRouter,
+	crate::routing::gossip::NetworkGraph,
+	crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters},
+	crate::sign::KeysManager,
+};
 
 use alloc::collections::{btree_map, BTreeMap};
 

lightning/src/ln/channelmanager.rs

@@ -43,10 +43,9 @@ use crate::ln::functional_test_utils;
 use crate::ln::functional_test_utils::*;
 use crate::routing::gossip::NodeId;
 #[cfg(feature = "std")]
-use {
-	crate::util::time::tests::SinceEpoch,
-	std::time::{SystemTime, Instant, Duration}
-};
+use std::time::{SystemTime, Instant, Duration};
+#[cfg(not(feature = "no-std"))]
+use crate::util::time::tests::SinceEpoch;
 
 #[test]
 fn mpp_failure() {

lightning/src/ln/payment_tests.rs

@@ -18,7 +18,7 @@
 use bitcoin::blockdata::constants::ChainHash;
 use bitcoin::secp256k1::{self, Secp256k1, SecretKey, PublicKey};
 
-use crate::sign::{KeysManager, NodeSigner, Recipient};
+use crate::sign::{NodeSigner, Recipient};
 use crate::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
 use crate::ln::ChannelId;
 use crate::ln::features::{InitFeatures, NodeFeatures};
@@ -33,20 +33,27 @@ use crate::ln::wire::{Encode, Type};
 #[cfg(not(c_bindings))]
 use crate::onion_message::{SimpleArcOnionMessenger, SimpleRefOnionMessenger};
 use crate::onion_message::{CustomOnionMessageHandler, OffersMessage, OffersMessageHandler, OnionMessageContents, PendingOnionMessage};
-use crate::routing::gossip::{NetworkGraph, P2PGossipSync, NodeId, NodeAlias};
+use crate::routing::gossip::{NodeId, NodeAlias};
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::logger::{Logger, WithContext};
 use crate::util::string::PrintableString;
 
 use crate::prelude::*;
 use crate::io;
 use alloc::collections::VecDeque;
-use crate::sync::{Arc, Mutex, MutexGuard, FairRwLock};
+use crate::sync::{Mutex, MutexGuard, FairRwLock};
 use core::sync::atomic::{AtomicBool, AtomicU32, AtomicI32, Ordering};
 use core::{cmp, hash, fmt, mem};
 use core::ops::Deref;
 use core::convert::Infallible;
-#[cfg(feature = "std")] use std::error;
+#[cfg(feature = "std")]
+use std::error;
+#[cfg(not(c_bindings))]
+use {
+	crate::routing::gossip::{NetworkGraph, P2PGossipSync},
+	crate::sign::KeysManager,
+	crate::sync::Arc,
+};
 
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hashes::sha256::HashEngine as Sha256Engine;

lightning/src/ln/peer_handler.rs

@@ -15,16 +15,14 @@ use crate::chain::channelmonitor::{CLOSED_CHANNEL_UPDATE_ID, ChannelMonitor};
 use crate::sign::EntropySource;
 use crate::chain::transaction::OutPoint;
 use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
-use crate::ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, Retry, RecipientOnionFields};
+use crate::ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RecipientOnionFields};
 use crate::ln::msgs;
 use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
-use crate::routing::router::{RouteParameters, PaymentParameters};
 use crate::util::test_channel_signer::TestChannelSigner;
 use crate::util::test_utils;
 use crate::util::errors::APIError;
 use crate::util::ser::{Writeable, ReadableArgs};
 use crate::util::config::UserConfig;
-use crate::util::string::UntrustedString;
 
 use bitcoin::hash_types::BlockHash;
 
@@ -496,6 +494,9 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 
 #[cfg(feature = "std")]
 fn do_test_data_loss_protect(reconnect_panicing: bool, substantially_old: bool, not_stale: bool) {
+	use crate::routing::router::{RouteParameters, PaymentParameters};
+	use crate::ln::channelmanager::Retry;
+	use crate::util::string::UntrustedString;
 	// When we get a data_loss_protect proving we're behind, we immediately panic as the
 	// chain::Watch API requirements have been violated (e.g. the user restored from a backup). The
 	// panic message informs the user they should force-close without broadcasting, which is tested

lightning/src/ln/reload_tests.rs

@@ -19,13 +19,12 @@ use crate::blinded_path::BlindedPath;
 use crate::blinded_path::message::{advance_path_by_one, ForwardTlvs, ReceiveTlvs};
 use crate::blinded_path::utils;
 use crate::events::{Event, EventHandler, EventsProvider};
-use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient};
+use crate::sign::{EntropySource, NodeSigner, Recipient};
 #[cfg(not(c_bindings))]
 use crate::ln::channelmanager::{SimpleArcChannelManager, SimpleRefChannelManager};
 use crate::ln::features::{InitFeatures, NodeFeatures};
 use crate::ln::msgs::{self, OnionMessage, OnionMessageHandler, SocketAddress};
 use crate::ln::onion_utils;
-use crate::ln::peer_handler::IgnoringMessageHandler;
 use crate::routing::gossip::{NetworkGraph, NodeId};
 pub use super::packet::OnionMessageContents;
 use super::packet::ParsedOnionMessageContents;
@@ -37,9 +36,16 @@ use crate::util::ser::Writeable;
 use core::fmt;
 use core::ops::Deref;
 use crate::io;
-use crate::sync::{Arc, Mutex};
+use crate::sync::Mutex;
 use crate::prelude::*;
 
+#[cfg(not(c_bindings))]
+use {
+	crate::sign::KeysManager,
+	crate::ln::peer_handler::IgnoringMessageHandler,
+	crate::sync::Arc,
+};
+
 pub(super) const MAX_TIMER_TICKS: usize = 2;
 
 /// A sender, receiver and forwarder of [`OnionMessage`]s.
@@ -258,7 +264,7 @@ pub struct PendingOnionMessage<T: OnionMessageContents> {
 ///
 /// These are obtained when released from [`OnionMessenger`]'s handlers after which they are
 /// enqueued for sending.
-pub type PendingOnionMessage<T: OnionMessageContents> = (T, Destination, Option<BlindedPath>);
+pub type PendingOnionMessage<T> = (T, Destination, Option<BlindedPath>);
 
 pub(crate) fn new_pending_onion_message<T: OnionMessageContents>(
 	contents: T, destination: Destination, reply_path: Option<BlindedPath>