Merge pull request #3016 from lightning-signer/2024-04-ext-test

allow functional tests to be used externally with a dynamic signer factory

Author: TheBlueMatt

.github/workflows/build.yml

@@ -13,6 +13,20 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
+  ext-test:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout source code
+        uses: actions/checkout@v4
+      - name: Install Rust stable toolchain
+        run: |
+          curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
+          rustup override set stable
+      - name: Run externalized tests
+        run: |
+          cd ext-functional-test-demo
+          cargo test --verbose --color always
+          cargo test --verbose --color always --features test-broken
   build:
     strategy:
       fail-fast: false

.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",

ext-functional-test-demo/Cargo.toml

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

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

@@ -0,0 +1,61 @@
+fn main() {
+	println!("{} tests were exported", lightning::get_xtests().len());
+}
+
+#[cfg(test)]
+#[allow(unused)]
+mod tests {
+	use lightning::ln::functional_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!()
+		}
+	}
+
+	#[cfg(feature = "test-broken")]
+	#[test]
+	fn test_broken() {
+		SIGNER_FACTORY.set(Arc::new(BrokenSignerFactory()));
+		catch_unwind(|| fake_network_test()).unwrap_err();
+	}
+
+	#[cfg(not(feature = "test-broken"))]
+	#[test]
+	fn test_default_one() {
+		test_htlc_on_chain_success();
+	}
+
+	#[cfg(not(feature = "test-broken"))]
+	#[test]
+	fn test_default_all() {
+		let mut failed_tests = Vec::new();
+		for test in lightning::get_xtests() {
+			print!("Running test: {}", test.test_name);
+			let mut pass = catch_unwind(|| (test.test_fn)()).is_ok();
+			if test.should_panic {
+				pass = !pass;
+			}
+			if !pass {
+				failed_tests.push(test.test_name);
+			}
+		}
+		if !failed_tests.is_empty() {
+			println!("Failed tests:");
+			for test in failed_tests.iter() {
+				println!("- {}", test);
+			}
+		}
+		println!("Done with {} failures", failed_tests.len());
+		assert!(failed_tests.is_empty());
+	}
+}

fuzz/src/chanmon_consistency.rs

@@ -79,6 +79,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;
@@ -375,6 +376,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)
 	}
 

fuzz/src/full_stack.rs

@@ -75,6 +75,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};
@@ -439,7 +440,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(&[
@@ -509,7 +510,7 @@ impl SignerProvider for KeyProvider {
 					channel_keys_id,
 					channel_keys_id,
 				)
-			},
+			}),
 			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/Cargo.toml

@@ -18,9 +18,12 @@ proc-macro = true
 [features]
 
 [dependencies]
-syn = { version = "2.0.77", default-features = false, features = ["parsing", "printing", "proc-macro", "full"] }
-proc-macro2 = { version = "1.0.86", default-features = false, features = ["proc-macro"] }
+syn = { version = "2.0", default-features = false, features = ["parsing", "printing", "proc-macro", "full"] }
+proc-macro2 = { version = "1.0", default-features = false, features = ["proc-macro"] }
 quote = { version = "1.0", default-features = false, features = ["proc-macro"] }
 
+[dev-dependencies]
+inventory = "0.3"
+
 [lints]
 workspace = true

lightning-macros/src/lib.rs

@@ -22,9 +22,11 @@ extern crate alloc;
 
 use alloc::string::ToString;
 use proc_macro::{Delimiter, Group, TokenStream, TokenTree};
+use proc_macro2::TokenStream as TokenStream2;
 use quote::quote;
 use syn::spanned::Spanned;
 use syn::{parse, ImplItemFn, Token};
+use syn::{parse_macro_input, Item};
 
 fn add_async_method(mut parsed: ImplItemFn) -> TokenStream {
 	let output = quote! {
@@ -294,3 +296,107 @@ pub fn drop_legacy_field_definition(expr: TokenStream) -> TokenStream {
 	let out = syn::Expr::Struct(st);
 	quote! { #out }.into()
 }
+
+/// An exposed test.  This is a test that will run locally and also be
+/// made available to other crates that want to run it in their own context.
+///
+/// For example:
+/// ```rust
+/// use lightning_macros::xtest;
+///
+/// fn f1() {}
+///
+/// #[xtest(feature = "_externalize_tests")]
+/// pub fn test_f1() {
+///     f1();
+/// }
+/// ```
+///
+/// Which will include the module if we are testing or the `_test_utils` feature
+/// is on.
+#[proc_macro_attribute]
+pub fn xtest(attrs: TokenStream, item: TokenStream) -> TokenStream {
+	let attrs = parse_macro_input!(attrs as TokenStream2);
+	let input = parse_macro_input!(item as Item);
+
+	let expanded = match input {
+		Item::Fn(item_fn) => {
+			let (cfg_attr, submit_attr) = if attrs.is_empty() {
+				(quote! { #[cfg_attr(test, test)] }, quote! { #[cfg(not(test))] })
+			} else {
+				(
+					quote! { #[cfg_attr(test, test)] #[cfg(any(test, #attrs))] },
+					quote! { #[cfg(all(not(test), #attrs))] },
+				)
+			};
+
+			// Check that the function doesn't take args and returns nothing
+			if !item_fn.sig.inputs.is_empty()
+				|| !matches!(item_fn.sig.output, syn::ReturnType::Default)
+			{
+				return syn::Error::new_spanned(
+					item_fn.sig,
+					"xtest functions must not take arguments and must return nothing",
+				)
+				.to_compile_error()
+				.into();
+			}
+
+			// Check for #[should_panic] attribute
+			let should_panic =
+				item_fn.attrs.iter().any(|attr| attr.path().is_ident("should_panic"));
+
+			let fn_name = &item_fn.sig.ident;
+			let fn_name_str = fn_name.to_string();
+			quote! {
+				#cfg_attr
+				#item_fn
+
+				// We submit the test to the inventory only if we're not actually testing
+				#submit_attr
+				inventory::submit! {
+					crate::XTestItem {
+						test_fn: #fn_name,
+						test_name: #fn_name_str,
+						should_panic: #should_panic,
+					}
+				}
+			}
+		},
+		_ => {
+			return syn::Error::new_spanned(
+				input,
+				"xtest can only be applied to functions or modules",
+			)
+			.to_compile_error()
+			.into();
+		},
+	};
+
+	TokenStream::from(expanded)
+}
+
+/// Collects all externalized tests marked with `#[xtest]`
+/// into a vector of `XTestItem`s.  This vector can be
+/// retrieved by calling `get_xtests()`.
+#[proc_macro]
+pub fn xtest_inventory(_input: TokenStream) -> TokenStream {
+	let expanded = quote! {
+		/// An externalized test item, including the test function, name, and whether it is marked with `#[should_panic]`.
+		pub struct XTestItem {
+			pub test_fn: fn(),
+			pub test_name: &'static str,
+			pub should_panic: bool,
+		}
+
+		inventory::collect!(XTestItem);
+
+		pub fn get_xtests() -> Vec<&'static XTestItem> {
+			inventory::iter::<XTestItem>
+				.into_iter()
+				.collect()
+		}
+	};
+
+	TokenStream::from(expanded)
+}

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
 	}