Expose functional tests under _externalize_tests feature flag

Also, introduce TestSignerFactory, a factory for dynamic signers and ext-functional-test-demo crate for testing this machinery.

Author: devrandom

.gitignore

@@ -13,5 +13,6 @@ lightning-rapid-gossip-sync/res/full_graph.lngossip
 lightning-custom-message/target
 lightning-transaction-sync/target
 lightning-dns-resolver/target
+ext-functional-test-demo/target
 no-std-check/target
 msrv-no-dev-deps-check/target

Cargo.toml

@@ -21,6 +21,7 @@ members = [
 
 exclude = [
     "lightning-transaction-sync",
+    "ext-functional-test-demo",
     "no-std-check",
     "msrv-no-dev-deps-check",
     "bench",

ci/ci-tests.sh

@@ -112,6 +112,13 @@ cargo check --verbose --color always
 [ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
 popd
 
+echo -e "\n\Running functional tests from outside the workspace"
+pushd ext-functional-test-demo
+[ "$RUSTC_MINOR_VERSION" -lt 65 ] && cargo update -p regex --precise "1.9.6" --verbose
+cargo test --color always
+[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
+popd
+
 # Test that we can build downstream code with only the "release pins".
 pushd msrv-no-dev-deps-check
 PIN_RELEASE_DEPS

ext-functional-test-demo/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "ext-functional-tester"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+lightning = { path = "../lightning", features = ["_externalize_tests"] }

ext-functional-test-demo/src/main.rs

@@ -0,0 +1,31 @@
+fn main() {
+	println!("{} tests were exported", lightning::get_xtests().len());
+}
+
+#[cfg(test)]
+mod tests {
+	use lightning::util::dyn_signer::{DynKeysInterfaceTrait, DynSigner};
+	use lightning::util::test_utils::{TestSignerFactory, SIGNER_FACTORY};
+	use std::panic::catch_unwind;
+	use std::sync::Arc;
+	use std::time::Duration;
+
+	struct BrokenSignerFactory();
+
+	impl TestSignerFactory for BrokenSignerFactory {
+		fn make_signer(
+			&self, _seed: &[u8; 32], _now: Duration,
+		) -> Box<dyn DynKeysInterfaceTrait<EcdsaSigner = DynSigner>> {
+			panic!()
+		}
+	}
+
+	#[test]
+	fn test_functional() {
+		lightning::ln::functional_tests::test_insane_channel_opens();
+		lightning::ln::functional_tests::fake_network_test();
+
+		SIGNER_FACTORY.set(Arc::new(BrokenSignerFactory()));
+		catch_unwind(|| lightning::ln::functional_tests::fake_network_test()).unwrap_err();
+	}
+}

fuzz/src/chanmon_consistency.rs

@@ -81,6 +81,7 @@ use bitcoin::secp256k1::schnorr;
 use bitcoin::secp256k1::{self, Message, PublicKey, Scalar, Secp256k1, SecretKey};
 
 use lightning::io::Cursor;
+use lightning::util::dyn_signer::DynSigner;
 use std::cmp::{self, Ordering};
 use std::mem;
 use std::sync::atomic;
@@ -382,6 +383,7 @@ impl SignerProvider for KeyProvider {
 			channel_keys_id,
 		);
 		let revoked_commitment = self.make_enforcement_state_cell(keys.commitment_seed);
+		let keys = DynSigner::new(keys);
 		TestChannelSigner::new_with_revoked(keys, revoked_commitment, false)
 	}
 
@@ -390,6 +392,7 @@ impl SignerProvider for KeyProvider {
 
 		let inner: InMemorySigner = ReadableArgs::read(&mut reader, self)?;
 		let state = self.make_enforcement_state_cell(inner.commitment_seed);
+		let inner = DynSigner::new(inner);
 
 		Ok(TestChannelSigner::new_with_revoked(inner, state, false))
 	}

fuzz/src/full_stack.rs

@@ -76,6 +76,7 @@ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature};
 use bitcoin::secp256k1::schnorr;
 use bitcoin::secp256k1::{self, Message, PublicKey, Scalar, Secp256k1, SecretKey};
 
+use lightning::util::dyn_signer::DynSigner;
 use std::cell::RefCell;
 use std::cmp;
 use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering};
@@ -444,7 +445,7 @@ impl SignerProvider for KeyProvider {
 		let ctr = channel_keys_id[0];
 		let (inbound, state) = self.signer_state.borrow().get(&ctr).unwrap().clone();
 		TestChannelSigner::new_with_revoked(
-			if inbound {
+			DynSigner::new(if inbound {
 				InMemorySigner::new(
 					&secp_ctx,
 					SecretKey::from_slice(&[
@@ -516,7 +517,7 @@ impl SignerProvider for KeyProvider {
 					channel_keys_id,
 					channel_keys_id,
 				)
-			},
+			}),
 			state,
 			false,
 		)
@@ -525,6 +526,7 @@ impl SignerProvider for KeyProvider {
 	fn read_chan_signer(&self, mut data: &[u8]) -> Result<TestChannelSigner, DecodeError> {
 		let inner: InMemorySigner = ReadableArgs::read(&mut data, self)?;
 		let state = Arc::new(Mutex::new(EnforcementState::new()));
+		let inner = DynSigner::new(inner);
 
 		Ok(TestChannelSigner::new_with_revoked(inner, state, false))
 	}

lightning-background-processor/src/lib.rs

@@ -2550,6 +2550,8 @@ mod tests {
 				failure: PathFailure::OnPath { network_update: None },
 				path: path.clone(),
 				short_channel_id: Some(scored_scid),
+				error_code: None,
+				error_data: None,
 			});
 			let event = $receive.expect("PaymentPathFailed not handled within deadline");
 			match event {
@@ -2567,6 +2569,8 @@ mod tests {
 				failure: PathFailure::OnPath { network_update: None },
 				path: path.clone(),
 				short_channel_id: None,
+				error_code: None,
+				error_data: None,
 			});
 			let event = $receive.expect("PaymentPathFailed not handled within deadline");
 			match event {

lightning-macros/src/lib.rs

@@ -306,7 +306,7 @@ pub fn drop_legacy_field_definition(expr: TokenStream) -> TokenStream {
 ///
 /// fn f1() {}
 ///
-/// #[xtest(feature = "_test_utils")]
+/// #[xtest(feature = "_externalize_tests")]
 /// pub fn test_f1() {
 ///     f1();
 /// }
@@ -317,7 +317,7 @@ pub fn drop_legacy_field_definition(expr: TokenStream) -> TokenStream {
 /// ```rust
 /// use lightning_macros::xtest;
 ///
-/// #[xtest(feature = "_test_utils")]
+/// #[xtest(feature = "_externalize_tests")]
 /// pub mod tests {
 /// 	use super::*;
 ///

lightning/Cargo.toml

@@ -18,7 +18,7 @@ rustdoc-args = ["--cfg", "docsrs"]
 [features]
 # Internal test utilities exposed to other repo crates
 _test_utils = ["regex", "bitcoin/bitcoinconsensus", "lightning-types/_test_utils"]
-
+_externalize_tests = ["inventory", "_test_utils"]
 # 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 = []
@@ -48,10 +48,12 @@ regex = { version = "1.5.6", optional = true }
 backtrace = { version = "0.3", optional = true }
 
 libm = { version = "0.2", default-features = false }
+inventory = { version = "0.3", optional = true  }
 
 [dev-dependencies]
 regex = "1.5.6"
 lightning-types = { version = "0.3.0", path = "../lightning-types", features = ["_test_utils"] }
+lightning-macros = { path = "../lightning-macros" }
 
 [dev-dependencies.bitcoin]
 version = "0.32.2"

lightning/src/chain/chainmonitor.rs

@@ -469,7 +469,7 @@ where C::Target: chain::Filter,
 	}
 
 
-	#[cfg(test)]
+	#[cfg(any(test, feature = "_test_utils"))]
 	pub fn remove_monitor(&self, channel_id: &ChannelId) -> ChannelMonitor<ChannelSigner> {
 		self.monitors.write().unwrap().remove(channel_id).unwrap().monitor
 	}

lightning/src/chain/channelmonitor.rs

@@ -854,10 +854,7 @@ impl Readable for IrrevocablyResolvedHTLC {
 /// returned block hash and the the current chain and then reconnecting blocks to get to the
 /// best chain) upon deserializing the object!
 pub struct ChannelMonitor<Signer: EcdsaChannelSigner> {
-	#[cfg(test)]
 	pub(crate) inner: Mutex<ChannelMonitorImpl<Signer>>,
-	#[cfg(not(test))]
-	pub(super) inner: Mutex<ChannelMonitorImpl<Signer>>,
 }
 
 impl<Signer: EcdsaChannelSigner> Clone for ChannelMonitor<Signer> where Signer: Clone {
@@ -965,9 +962,9 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
 	// Obviously Correct (tm) if we just keep track of them explicitly.
 	outputs_to_watch: HashMap<Txid, Vec<(u32, ScriptBuf)>>,
 
-	#[cfg(test)]
+	#[cfg(any(test, feature = "_test_utils"))]
 	pub onchain_tx_handler: OnchainTxHandler<Signer>,
-	#[cfg(not(test))]
+	#[cfg(not(any(test, feature = "_test_utils")))]
 	onchain_tx_handler: OnchainTxHandler<Signer>,
 
 	// This is set when the Channel[Manager] generated a ChannelMonitorUpdate which indicated the
@@ -1814,7 +1811,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	/// Unsafe test-only version of `broadcast_latest_holder_commitment_txn` used by our test framework
 	/// to bypass HolderCommitmentTransaction state update lockdown after signature and generate
 	/// revoked commitment transaction.
-	#[cfg(any(test, feature = "unsafe_revoked_tx_signing"))]
+	#[cfg(any(test, feature = "_test_utils", feature = "unsafe_revoked_tx_signing"))]
 	pub fn unsafe_get_latest_holder_commitment_txn<L: Deref>(&self, logger: &L) -> Vec<Transaction>
 	where L::Target: Logger {
 		let mut inner = self.inner.lock().unwrap();
@@ -2128,7 +2125,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 		self.inner.lock().unwrap().counterparty_payment_script = script;
 	}
 
-	#[cfg(test)]
+	#[cfg(any(test, feature = "_test_utils"))]
 	pub fn do_mut_signer_call<F: FnMut(&mut Signer) -> ()>(&self, mut f: F) {
 		let mut inner = self.inner.lock().unwrap();
 		f(&mut inner.onchain_tx_handler.signer);
@@ -2769,7 +2766,7 @@ macro_rules! fail_unbroadcast_htlcs {
 // witness length match (ie is 136 bytes long). We generate one here which we also use in some
 // in-line tests later.
 
-#[cfg(test)]
+#[cfg(any(test, feature = "_test_utils"))]
 pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {
 	use bitcoin::opcodes;
 	let mut ret = [opcodes::all::OP_NOP.to_u8(); 136];
@@ -2781,7 +2778,7 @@ pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {
 	Vec::from(&ret[..])
 }
 
-#[cfg(test)]
+#[cfg(any(test, feature = "_test_utils"))]
 pub fn deliberately_bogus_accepted_htlc_witness() -> Vec<Vec<u8>> {
 	vec![Vec::new(), Vec::new(), Vec::new(), Vec::new(), deliberately_bogus_accepted_htlc_witness_program().into()].into()
 }
@@ -3947,7 +3944,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		}
 	}
 
-	#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
+	#[cfg(any(test, feature = "_test_utils", feature = "unsafe_revoked_tx_signing"))]
 	/// Note that this includes possibly-locktimed-in-the-future transactions!
 	fn unsafe_get_latest_holder_commitment_txn<L: Deref>(
 		&mut self, logger: &WithChannelMonitor<L>
@@ -5289,7 +5286,7 @@ mod tests {
 		nodes[1].chain_monitor.chain_monitor.transactions_confirmed(&new_header,
 			&[(0, broadcast_tx)], conf_height);
 
-		let (_, pre_update_monitor) = <(BlockHash, ChannelMonitor<InMemorySigner>)>::read(
+		let (_, pre_update_monitor) = <(BlockHash, ChannelMonitor<_>)>::read(
 						&mut io::Cursor::new(&get_monitor!(nodes[1], channel.2).encode()),
 						(&nodes[1].keys_manager.backing, &nodes[1].keys_manager.backing)).unwrap();
 

lightning/src/chain/onchaintx.rs

@@ -250,9 +250,9 @@ pub struct OnchainTxHandler<ChannelSigner: EcdsaChannelSigner> {
 	// Key is identifier of the pending claim request, i.e the txid of the initial claiming transaction generated by
 	// us and is immutable until all outpoint of the claimable set are post-anti-reorg-delay solved.
 	// Entry is cache of elements need to generate a bumped claiming transaction (see ClaimTxBumpMaterial)
-	#[cfg(test)] // Used in functional_test to verify sanitization
+	#[cfg(any(test, feature = "_test_utils"))]
 	pub(crate) pending_claim_requests: HashMap<ClaimId, PackageTemplate>,
-	#[cfg(not(test))]
+	#[cfg(not(any(test, feature = "_test_utils")))]
 	pending_claim_requests: HashMap<ClaimId, PackageTemplate>,
 
 	// Used to track external events that need to be forwarded to the `ChainMonitor`. This `Vec`
@@ -273,9 +273,9 @@ pub struct OnchainTxHandler<ChannelSigner: EcdsaChannelSigner> {
 	// block height, and are immutable until the outpoint has enough confirmations to meet our
 	// [`ANTI_REORG_DELAY`]. The initial confirmation block height is used to remove the entry if
 	// the block gets disconnected.
-	#[cfg(test)] // Used in functional_test to verify sanitization
-	pub claimable_outpoints: HashMap<BitcoinOutPoint, (ClaimId, u32)>,
-	#[cfg(not(test))]
+	#[cfg(any(test, feature = "_test_utils"))]
+	pub(crate) claimable_outpoints: HashMap<BitcoinOutPoint, (ClaimId, u32)>,
+	#[cfg(not(any(test, feature = "_test_utils")))]
 	claimable_outpoints: HashMap<BitcoinOutPoint, (ClaimId, u32)>,
 
 	locktimed_packages: BTreeMap<u32, Vec<PackageTemplate>>,
@@ -1194,7 +1194,7 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 		MaybeSignedTransaction(tx)
 	}
 
-	#[cfg(any(test, feature="unsafe_revoked_tx_signing"))]
+	#[cfg(any(test, feature="_test_utils", feature="unsafe_revoked_tx_signing"))]
 	pub(crate) fn get_fully_signed_copy_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction {
 		let sig = self.signer.unsafe_sign_holder_commitment(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment");
 		self.holder_commitment.add_holder_sig(funding_redeemscript, sig)

lightning/src/chain/package.rs

@@ -105,10 +105,10 @@ pub(crate) fn verify_channel_type_features(channel_type_features: &Option<Channe
 // number_of_witness_elements + sig_length + revocation_sig + true_length + op_true + witness_script_length + witness_script
 pub(crate) const WEIGHT_REVOKED_OUTPUT: u64 = 1 + 1 + 73 + 1 + 1 + 1 + 77;
 
-#[cfg(not(test))]
+#[cfg(not(any(test, feature = "_test_utils")))]
 /// Height delay at which transactions are fee-bumped/rebroadcasted with a low priority.
 const LOW_FREQUENCY_BUMP_INTERVAL: u32 = 15;
-#[cfg(test)]
+#[cfg(any(test, feature = "_test_utils"))]
 /// Height delay at which transactions are fee-bumped/rebroadcasted with a low priority.
 pub(crate) const LOW_FREQUENCY_BUMP_INTERVAL: u32 = 15;
 

lightning/src/events/mod.rs

@@ -1054,9 +1054,9 @@ pub enum Event {
 		/// If this is `Some`, then the corresponding channel should be avoided when the payment is
 		/// retried. May be `None` for older [`Event`] serializations.
 		short_channel_id: Option<u64>,
-#[cfg(test)]
+		#[cfg(any(test, feature = "_test_utils"))]
 		error_code: Option<u16>,
-#[cfg(test)]
+		#[cfg(any(test, feature = "_test_utils"))]
 		error_data: Option<Vec<u8>>,
 	},
 	/// Indicates that a probe payment we sent returned successful, i.e., only failed at the destination.
@@ -1569,15 +1569,15 @@ impl Writeable for Event {
 			&Event::PaymentPathFailed {
 				ref payment_id, ref payment_hash, ref payment_failed_permanently, ref failure,
 				ref path, ref short_channel_id,
-				#[cfg(test)]
+				#[cfg(any(test, feature = "_test_utils"))]
 				ref error_code,
-				#[cfg(test)]
+				#[cfg(any(test, feature = "_test_utils"))]
 				ref error_data,
 			} => {
 				3u8.write(writer)?;
-				#[cfg(test)]
+				#[cfg(any(test, feature = "_test_utils"))]
 				error_code.write(writer)?;
-				#[cfg(test)]
+				#[cfg(any(test, feature = "_test_utils"))]
 				error_data.write(writer)?;
 				write_tlv_fields!(writer, {
 					(0, payment_hash, required),
@@ -1920,9 +1920,9 @@ impl MaybeReadable for Event {
 			},
 			3u8 => {
 				let mut f = || {
-					#[cfg(test)]
+					#[cfg(any(test, feature = "_test_utils"))]
 					let error_code = Readable::read(reader)?;
-					#[cfg(test)]
+					#[cfg(any(test, feature = "_test_utils"))]
 					let error_data = Readable::read(reader)?;
 					let mut payment_hash = PaymentHash([0; 32]);
 					let mut payment_failed_permanently = false;
@@ -1952,9 +1952,9 @@ impl MaybeReadable for Event {
 						failure,
 						path: Path { hops: path.unwrap(), blinded_tail },
 						short_channel_id,
-						#[cfg(test)]
+						#[cfg(any(test, feature = "_test_utils"))]
 						error_code,
-						#[cfg(test)]
+						#[cfg(any(test, feature = "_test_utils"))]
 						error_data,
 					}))
 				};
@@ -2364,7 +2364,7 @@ impl MaybeReadable for Event {
 /// broadcast to most peers).
 /// These events are handled by PeerManager::process_events if you are using a PeerManager.
 #[derive(Clone, Debug)]
-#[cfg_attr(test, derive(PartialEq))]
+#[cfg_attr(any(test, feature = "_test_utils"), derive(PartialEq))]
 pub enum MessageSendEvent {
 	/// Used to indicate that we've accepted a channel open and should send the accept_channel
 	/// message provided to the given peer.

lightning/src/lib.rs

@@ -142,3 +142,6 @@ mod prelude {
 extern crate backtrace;
 
 mod sync;
+
+#[cfg(feature = "_externalize_tests")]
+lightning_macros::xtest_inventory!();