Merge pull request lightningdevkit#3249 from TheBlueMatt/2024-08-feature-cleanup-1

TheBlueMatt · web-flow · commit 37154109c17e · 2024-08-19T18:50:52.000Z
bitcoin 0.32 upgrade followups
diff --git a/Cargo.toml b/Cargo.toml
@@ -41,9 +41,6 @@ opt-level = 3
 lto = true
 panic = "abort"
 
-[patch.crates-io.possiblyrandom]
-path = "possiblyrandom"
-
 [workspace.lints.rust.unexpected_cfgs]
 level = "forbid"
 # When adding a new cfg attribute, ensure that it is added to this list.
@@ -58,6 +55,7 @@ check-cfg = [
     "cfg(debug_assertions)",
     "cfg(c_bindings)",
     "cfg(ldk_bench)",
+    "cfg(ldk_test_vectors)",
     "cfg(taproot)",
     "cfg(async_signing)",
     "cfg(require_route_graph_test)",
diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh
@@ -32,83 +32,79 @@ cargo test --verbose --color always
 cargo check --verbose --color always
 
 echo -e "\n\nBuilding and testing Block Sync Clients with features"
-pushd lightning-block-sync
-cargo test --verbose --color always --features rest-client
-cargo check --verbose --color always --features rest-client
-cargo test --verbose --color always --features rpc-client
-cargo check --verbose --color always --features rpc-client
-cargo test --verbose --color always --features rpc-client,rest-client
-cargo check --verbose --color always --features rpc-client,rest-client
-cargo test --verbose --color always --features rpc-client,rest-client,tokio
-cargo check --verbose --color always --features rpc-client,rest-client,tokio
-popd
+
+cargo test -p lightning-block-sync --verbose --color always --features rest-client
+cargo check -p lightning-block-sync --verbose --color always --features rest-client
+cargo test -p lightning-block-sync --verbose --color always --features rpc-client
+cargo check -p lightning-block-sync --verbose --color always --features rpc-client
+cargo test -p lightning-block-sync --verbose --color always --features rpc-client,rest-client
+cargo check -p lightning-block-sync --verbose --color always --features rpc-client,rest-client
+cargo test -p lightning-block-sync --verbose --color always --features rpc-client,rest-client,tokio
+cargo check -p lightning-block-sync --verbose --color always --features rpc-client,rest-client,tokio
 
 if [[ "$HOST_PLATFORM" != *windows* ]]; then
-	pushd lightning-transaction-sync
 	echo -e "\n\nChecking Transaction Sync Clients with features."
-	cargo check --verbose --color always --features esplora-blocking
-	cargo check --verbose --color always --features esplora-async
-	cargo check --verbose --color always --features esplora-async-https
-	cargo check --verbose --color always --features electrum
+	cargo check -p lightning-transaction-sync --verbose --color always --features esplora-blocking
+	cargo check -p lightning-transaction-sync --verbose --color always --features esplora-async
+	cargo check -p lightning-transaction-sync --verbose --color always --features esplora-async-https
+	cargo check -p lightning-transaction-sync --verbose --color always --features electrum
 
 	if [ -z "$CI_ENV" ] && [[ -z "$BITCOIND_EXE" || -z "$ELECTRS_EXE" ]]; then
 		echo -e "\n\nSkipping testing Transaction Sync Clients due to BITCOIND_EXE or ELECTRS_EXE being unset."
-		cargo check --tests
+		cargo check -p lightning-transaction-sync --tests
 	else
 		echo -e "\n\nTesting Transaction Sync Clients with features."
-		cargo test --verbose --color always --features esplora-blocking
-		cargo test --verbose --color always --features esplora-async
-		cargo test --verbose --color always --features esplora-async-https
-		cargo test --verbose --color always --features electrum
+		cargo test -p lightning-transaction-sync --verbose --color always --features esplora-blocking
+		cargo test -p lightning-transaction-sync --verbose --color always --features esplora-async
+		cargo test -p lightning-transaction-sync --verbose --color always --features esplora-async-https
+		cargo test -p lightning-transaction-sync --verbose --color always --features electrum
 	fi
-	popd
 fi
 
 echo -e "\n\nTest futures builds"
-pushd lightning-background-processor
-cargo test --verbose --color always --features futures
-popd
+cargo test -p lightning-background-processor --verbose --color always --features futures
+cargo test -p lightning-background-processor --verbose --color always --features futures --no-default-features
 
 echo -e "\n\nTest Custom Message Macros"
-pushd lightning-custom-message
-cargo test --verbose --color always
+cargo test -p lightning-custom-message --verbose --color always
 [ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
-popd
 
 echo -e "\n\nTest backtrace-debug builds"
-pushd lightning
-cargo test --verbose --color always --features backtrace
-popd
+cargo test -p lightning --verbose --color always --features backtrace
 
 echo -e "\n\nBuilding with all Log-Limiting features"
-pushd lightning
-grep '^max_level_' Cargo.toml | awk '{ print $1 }'| while read -r FEATURE; do
-	RUSTFLAGS="$RUSTFLAGS -A unused_variables -A unused_macros -A unused_imports -A dead_code" cargo check --verbose --color always --features "$FEATURE"
+grep '^max_level_' lightning/Cargo.toml | awk '{ print $1 }'| while read -r FEATURE; do
+	RUSTFLAGS="$RUSTFLAGS -A unused_variables -A unused_macros -A unused_imports -A dead_code" cargo check -p lightning --verbose --color always --features "$FEATURE"
 done
-popd
 
-echo -e "\n\nTesting no-std flags in various combinations"
-for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do
-	cargo test -p $DIR --verbose --color always --no-default-features --features no-std
-	# check if there is a conflict between no-std and the default std feature
-	cargo test -p $DIR --verbose --color always --features no-std
+echo -e "\n\nTesting no-std builds"
+for DIR in lightning-invoice lightning-rapid-gossip-sync; do
+	cargo test -p $DIR --verbose --color always --no-default-features
 done
 
-for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do
+cargo test -p lightning --verbose --color always --no-default-features --features no-std
+# check if there is a conflict between no-std and the default std feature
+cargo test -p lightning --verbose --color always --features no-std
+
+echo -e "\n\nTesting c_bindings builds"
+RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test --verbose --color always
+
+for DIR in lightning-invoice lightning-rapid-gossip-sync; do
 	# check if there is a conflict between no-std and the c_bindings cfg
-	RUSTFLAGS="$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
 done
-RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test --verbose --color always
 
+# Note that because `$RUSTFLAGS` is not passed through to doctest builds we cannot selectively
+# disable tests in `c_bindings` so we skip doctests entirely here.
+RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test -p lightning-background-processor --verbose --color always --features futures --no-default-features --lib --bins --tests
+RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test -p lightning --verbose --color always --no-default-features --features=no-std
+
+echo -e "\n\nTesting other crate-specific builds"
 # Note that outbound_commitment_test only runs in this mode because of hardcoded signature values
-pushd lightning
-cargo test --verbose --color always --no-default-features --features=std,_test_vectors
-popd
+RUSTFLAGS="$RUSTFLAGS --cfg=ldk_test_vectors" cargo test -p lightning --verbose --color always --no-default-features --features=std
 # This one only works for lightning-invoice
-pushd lightning-invoice
 # check that compile with no-std and serde works in lightning-invoice
-cargo test --verbose --color always --no-default-features --features no-std --features serde
-popd
+cargo test -p lightning-invoice --verbose --color always --no-default-features --features serde
 
 echo -e "\n\nTesting no-std build on a downstream no-std crate"
 # check no-std compatibility across dependencies
diff --git a/lightning-background-processor/Cargo.toml b/lightning-background-processor/Cargo.toml
@@ -15,8 +15,7 @@ rustdoc-args = ["--cfg", "docsrs"]
 
 [features]
 futures = [ ]
-std = ["bitcoin/std", "lightning/std", "lightning-rapid-gossip-sync/std"]
-no-std = ["lightning/no-std", "lightning-rapid-gossip-sync/no-std"]
+std = []
 
 default = ["std"]
 
diff --git a/lightning-invoice/Cargo.toml b/lightning-invoice/Cargo.toml
@@ -15,9 +15,7 @@ all-features = true
 rustdoc-args = ["--cfg", "docsrs"]
 
 [features]
-default = ["std"]
-no-std = []
-std = ["bech32/std"]
+std = []
 
 [dependencies]
 bech32 = { version = "0.9.1", default-features = false }
diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs
@@ -22,9 +22,6 @@
 //!
 //! [`Bolt11Invoice::from_str`]: crate::Bolt11Invoice#impl-FromStr
 
-#[cfg(not(any(feature = "std", feature = "no-std")))]
-compile_error!("at least one of the `std` or `no-std` features must be enabled");
-
 extern crate bech32;
 extern crate lightning_types;
 extern crate alloc;
diff --git a/lightning-rapid-gossip-sync/Cargo.toml b/lightning-rapid-gossip-sync/Cargo.toml
@@ -11,8 +11,7 @@ Utility to process gossip routing data from Rapid Gossip Sync Server.
 
 [features]
 default = ["std"]
-no-std = ["lightning/no-std"]
-std = ["lightning/std"]
+std = []
 
 [dependencies]
 lightning = { version = "0.0.123-beta", path = "../lightning", default-features = false }
diff --git a/lightning-rapid-gossip-sync/src/lib.rs b/lightning-rapid-gossip-sync/src/lib.rs
@@ -143,7 +143,7 @@ where
 	///
 	/// `sync_path`: Path to the file where the gossip update data is located
 	///
-	#[cfg(all(feature = "std", not(feature = "no-std")))]
+	#[cfg(feature = "std")]
 	pub fn sync_network_graph_with_file_path(
 		&self, sync_path: &str,
 	) -> Result<u32, GraphSyncError> {
@@ -191,7 +191,7 @@ where
 	}
 }
 
-#[cfg(all(feature = "std", not(feature = "no-std")))]
+#[cfg(feature = "std")]
 #[cfg(test)]
 mod tests {
 	use std::fs;
diff --git a/lightning/Cargo.toml b/lightning/Cargo.toml
@@ -28,11 +28,9 @@ max_level_trace = []
 # Allow signing of local transactions that may have been revoked or will be revoked, for functional testing (e.g. justice tx handling).
 # This is unsafe to use in production because it may result in the counterparty publishing taking our funds.
 unsafe_revoked_tx_signing = []
-# Override signing to not include randomness when generating signatures for test vectors.
-_test_vectors = []
 
-no-std = ["hashbrown", "possiblyrandom", "lightning-invoice/no-std", "libm"]
-std = ["lightning-invoice/std", "bech32/std"]
+no-std = ["hashbrown", "possiblyrandom", "libm"]
+std = []
 
 # Generates low-r bitcoin signatures, which saves 1 byte in 50% of the cases
 grind_signatures = []
@@ -47,7 +45,7 @@ bech32 = { version = "0.9.1", default-features = false }
 bitcoin = { version = "0.32.2", default-features = false, features = ["secp-recovery"] }
 
 hashbrown = { version = "0.13", optional = true, default-features = false }
-possiblyrandom = { version = "0.2", optional = true, default-features = false }
+possiblyrandom = { version = "0.2", path = "../possiblyrandom", optional = true, default-features = false }
 regex = { version = "1.5.6", optional = true }
 backtrace = { version = "0.3", optional = true }
 
diff --git a/lightning/src/crypto/utils.rs b/lightning/src/crypto/utils.rs
@@ -75,9 +75,9 @@ pub fn sign_with_aux_rand<C: Signing, ES: Deref>(
 			break sig;
 		}
 	};
-	#[cfg(all(not(feature = "grind_signatures"), not(feature = "_test_vectors")))]
+	#[cfg(all(not(feature = "grind_signatures"), not(ldk_test_vectors)))]
 	let sig = ctx.sign_ecdsa_with_noncedata(msg, sk, &entropy_source.get_secure_random_bytes());
-	#[cfg(all(not(feature = "grind_signatures"), feature = "_test_vectors"))]
+	#[cfg(all(not(feature = "grind_signatures"), ldk_test_vectors))]
 	let sig = sign(ctx, msg, sk);
 	sig
 }
diff --git a/lightning/src/ln/bolt11_payment.rs b/lightning/src/ln/bolt11_payment.rs
@@ -188,11 +188,12 @@ mod tests {
 
 		let secp_ctx = Secp256k1::new();
 		let node_secret = nodes[1].keys_manager.backing.get_node_secret_key();
+		let time = std::time::SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
 		let invoice = InvoiceBuilder::new(Currency::Bitcoin)
 			.description("test".into())
 			.payment_hash(Sha256::from_slice(&payment_hash.0).unwrap())
 			.payment_secret(payment_secret)
-			.current_timestamp()
+			.duration_since_epoch(time)
 			.min_final_cltv_expiry_delta(144)
 			.amount_milli_satoshis(50_000)
 			.payment_metadata(payment_metadata.clone())
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -9659,8 +9659,9 @@ mod tests {
 		}
 	}
 
-	#[cfg(all(feature = "_test_vectors", not(feature = "grind_signatures")))]
+	#[cfg(ldk_test_vectors)]
 	fn public_from_secret_hex(secp_ctx: &Secp256k1<bitcoin::secp256k1::All>, hex: &str) -> PublicKey {
+		assert!(cfg!(not(feature = "grind_signatures")));
 		PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&<Vec<u8>>::from_hex(hex).unwrap()[..]).unwrap())
 	}
 
@@ -10237,9 +10238,11 @@ mod tests {
 		assert_eq!(decoded_chan.context.holding_cell_htlc_updates, holding_cell_htlc_updates);
 	}
 
-	#[cfg(all(feature = "_test_vectors", not(feature = "grind_signatures")))]
+	#[cfg(ldk_test_vectors)]
 	#[test]
 	fn outbound_commitment_test() {
+		assert!(cfg!(not(feature = "grind_signatures")));
+
 		use bitcoin::sighash;
 		use bitcoin::consensus::encode::serialize;
 		use bitcoin::sighash::EcdsaSighashType;
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -532,9 +532,9 @@ impl core::hash::Hash for HTLCSource {
 	}
 }
 impl HTLCSource {
-	#[cfg(all(feature = "_test_vectors", not(feature = "grind_signatures")))]
-	#[cfg(test)]
+	#[cfg(all(ldk_test_vectors, test))]
 	pub fn dummy() -> Self {
+		assert!(cfg!(not(feature = "grind_signatures")));
 		HTLCSource::OutboundRoute {
 			path: Path { hops: Vec::new(), blinded_tail: None },
 			session_priv: SecretKey::from_slice(&[1; 32]).unwrap(),
diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
@@ -2990,7 +2990,7 @@ fn test_anchors_monitor_fixes_counterparty_payment_script_on_reload() {
 	do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(true);
 }
 
-#[cfg(not(feature = "_test_vectors"))]
+#[cfg(not(ldk_test_vectors))]
 fn do_test_monitor_claims_with_random_signatures(anchors: bool, confirm_counterparty_commitment: bool) {
 	// Tests that our monitor claims will always use fresh random signatures (ensuring a unique
 	// wtxid) to prevent certain classes of transaction replacement at the bitcoin P2P layer.
@@ -3089,7 +3089,7 @@ fn do_test_monitor_claims_with_random_signatures(anchors: bool, confirm_counterp
 	}
 }
 
-#[cfg(not(feature = "_test_vectors"))]
+#[cfg(not(ldk_test_vectors))]
 #[test]
 fn test_monitor_claims_with_random_signatures() {
 	do_test_monitor_claims_with_random_signatures(false, false);
diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs
@@ -66,6 +66,7 @@ impl<W: Write> Writer for W {
 	}
 }
 
+// TODO: Drop this entirely if rust-bitcoin releases a version bump with https://github.com/rust-bitcoin/rust-bitcoin/pull/3173
 /// Wrap buffering support for implementations of Read.
 /// A [`Read`]er which keeps an internal buffer to avoid hitting the underlying stream directly for
 /// every read, implementing [`BufRead`].
@@ -92,17 +93,21 @@ impl<'a, R: Read> BufReader<'a, R> {
 impl<'a, R: Read> Read for BufReader<'a, R> {
 	#[inline]
 	fn read(&mut self, output: &mut [u8]) -> io::Result<usize> {
-		let input = self.fill_buf()?;
-		let count = cmp::min(input.len(), output.len());
-		output[..count].copy_from_slice(&input[..count]);
-		self.consume(count);
-		Ok(count)
+		if output.is_empty() { return Ok(0); }
+		let mut offset = 0;
+		if !self.is_consumed {
+			output[0] = self.buf[0];
+			self.is_consumed = true;
+			offset = 1;
+		}
+		self.inner.read(&mut output[offset..]).map(|len| len + offset)
 	}
 }
 
 impl<'a, R: Read> BufRead for BufReader<'a, R> {
 	#[inline]
 	fn fill_buf(&mut self) -> io::Result<&[u8]> {
+		debug_assert!(false, "rust-bitcoin doesn't actually use this");
 		if self.is_consumed {
 			let count = self.inner.read(&mut self.buf[..])?;
 			debug_assert!(count <= 1, "read gave us a garbage length");
@@ -111,11 +116,16 @@ impl<'a, R: Read> BufRead for BufReader<'a, R> {
 			self.is_consumed = count == 0;
 		}
 
-		Ok(&self.buf[..])
+		if self.is_consumed {
+			Ok(&[])
+		} else {
+			Ok(&self.buf[..])
+		}
 	}
 
 	#[inline]
 	fn consume(&mut self, amount: usize) {
+		debug_assert!(false, "rust-bitcoin doesn't actually use this");
 		if amount >= 1 {
 			debug_assert_eq!(amount, 1, "Can only consume one byte");
 			debug_assert!(!self.is_consumed, "Cannot consume more than had been read");
diff --git a/no-std-check/Cargo.toml b/no-std-check/Cargo.toml
@@ -4,7 +4,7 @@ version = "0.1.0"
 edition = "2021"
 
 [features]
-default = ["lightning/no-std", "lightning-invoice/no-std", "lightning-rapid-gossip-sync/no-std"]
+default = ["lightning/no-std"]
 
 [dependencies]
 lightning = { path = "../lightning", default-features = false }
@@ -15,6 +15,3 @@ lightning-background-processor = { path = "../lightning-background-processor", f
 # Obviously lightning-transaction-sync doesn't support no-std, but it should build
 # even if lightning is built with no-std.
 lightning-transaction-sync = { path = "../lightning-transaction-sync", optional = true }
-
-[patch.crates-io]
-possiblyrandom = { path = "../possiblyrandom" }

Original file line number	Diff line number	Diff line change
`@@ -75,9 +75,9 @@ pub fn sign_with_aux_rand<C: Signing, ES: Deref>(`
`75`	`75`	`break sig;`
`76`	`76`	`}`
`77`	`77`	`};`
`78`		`- #[cfg(all(not(feature = "grind_signatures"), not(feature = "_test_vectors")))]`
	`78`	`+ #[cfg(all(not(feature = "grind_signatures"), not(ldk_test_vectors)))]`
`79`	`79`	`let sig = ctx.sign_ecdsa_with_noncedata(msg, sk, &entropy_source.get_secure_random_bytes());`
`80`		`- #[cfg(all(not(feature = "grind_signatures"), feature = "_test_vectors"))]`
	`80`	`+ #[cfg(all(not(feature = "grind_signatures"), ldk_test_vectors))]`
`81`	`81`	`let sig = sign(ctx, msg, sk);`
`82`	`82`	`sig`
`83`	`83`	`}`