From aadf233d6c2974672e061d2ae755ac1f238812c9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 16 Aug 2024 18:46:24 +0000 Subject: [PATCH 1/9] Fix bug in `BufReader::fill_buf` when reaching EOF When we reach EOF we may return a full buffer when we should return an empty one. --- lightning/src/util/ser.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 19281110d6a..fbe1933a0cb 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -111,7 +111,11 @@ 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] From f8b7ad242dcadc610c7da9a08b4861b2fa396512 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 16 Aug 2024 18:47:38 +0000 Subject: [PATCH 2/9] Optimize `BufReader` somewhat for the only used case `rust-bitcoin` doesn't ever actually *use* its `BufRead` requirement when deserializing objects, and forcing it is somewhat inefficient, so we optimize the only (actual) case here by passing reads straight through to the backing stream. --- lightning/src/util/ser.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index fbe1933a0cb..de54a14f2f7 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -66,6 +66,7 @@ impl 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 { - 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"); @@ -120,6 +125,7 @@ impl<'a, R: Read> BufRead for BufReader<'a, R> { #[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"); From 9ca38c59f426034e889ad9a19d0736978c718b32 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 16 Aug 2024 20:13:18 +0000 Subject: [PATCH 3/9] Ensure we always pass a `path` for in-workspace dependencies In order to ensure our crates depend on the workspace copies of each other in test builds we need to override the crates.io dependency with a local `path`. We can do this in one of two ways - either specify the `path` in the dependency listing in each crate's `Cargo.toml` or use the workspace `Cargo.toml` to `patch` all dependencies. The first is tedious while the second lets us have it all in one place. However, the second option does break `cargo *` in individual crate directories (forcing the use of `cargo -p crate *` instead) and makes it rather difficult to depend on local versions of workspace crates. Thus, here we drop the `patch.crates-io` from our top-level `Cargo.toml` entirely. Still, we do update the `ci/ci-tests.sh` script here to use `cargo -p crate` instead of switching to each crate's directory as it allows `cargo` to use a shared `target` and may speed up tests. --- Cargo.toml | 3 -- ci/ci-tests.sh | 65 ++++++++++++++++------------------------- lightning/Cargo.toml | 2 +- no-std-check/Cargo.toml | 3 -- 4 files changed, 26 insertions(+), 47 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f7f4ba021fa..d1e2f0e054a 100644 --- 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. diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh index 43d6aeaa46f..ff955666cc3 100755 --- a/ci/ci-tests.sh +++ b/ci/ci-tests.sh @@ -32,60 +32,49 @@ 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 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 @@ -101,14 +90,10 @@ done 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 -cargo test --verbose --color always --no-default-features --features=std,_test_vectors -popd +cargo test -p lightning --verbose --color always --no-default-features --features=std,_test_vectors # 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 no-std --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/Cargo.toml b/lightning/Cargo.toml index 9f310c12dbc..6fab8d5ecf3 100644 --- a/lightning/Cargo.toml +++ b/lightning/Cargo.toml @@ -47,7 +47,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/no-std-check/Cargo.toml b/no-std-check/Cargo.toml index 45a70c2a6d1..c9d404c922f 100644 --- a/no-std-check/Cargo.toml +++ b/no-std-check/Cargo.toml @@ -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" } From 5e370742463be8f8d534d51c97d9331253059b8f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 16 Aug 2024 19:07:05 +0000 Subject: [PATCH 4/9] Drop the `no-std` feature from BP and drop feature implications Now that we don't have to have everything in our entire ecosystem use the same `std`/`no-std` feature combinations we should start by untangling our own features a bit. This takes the first step by removing the `no-std` feature entirely from the `lightning-background-processor` crate and removing most feature implications on dependencies from the remaining `std` feature. It also addresses a CI oversight where we were not testing `lightning-background-processor` without the `std` feature in CI at all. --- ci/ci-tests.sh | 1 + lightning-background-processor/Cargo.toml | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh index ff955666cc3..13ddcaf9a5b 100755 --- a/ci/ci-tests.sh +++ b/ci/ci-tests.sh @@ -63,6 +63,7 @@ fi echo -e "\n\nTest futures builds" 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" cargo test -p lightning-custom-message --verbose --color always diff --git a/lightning-background-processor/Cargo.toml b/lightning-background-processor/Cargo.toml index c780cc3d64b..741e62438a1 100644 --- 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"] From 31a9cd2d3d339aaf0dd28ca4ba68b42bcce8b65e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 16 Aug 2024 19:17:42 +0000 Subject: [PATCH 5/9] Drop the `no-std` feature from `lightning-invoice` Now that we don't have to have everything in our entire ecosystem use the same `std`/`no-std` feature combinations we should start by untangling our own features a bit. This takes another step by removing the `no-std` feature entirely from the `lightning-invoice` crate and removing all feature implications on dependencies from the remaining `std` feature. --- ci/ci-tests.sh | 13 ++++++++++--- lightning-invoice/Cargo.toml | 4 +--- lightning-invoice/src/lib.rs | 3 --- lightning/Cargo.toml | 2 +- no-std-check/Cargo.toml | 2 +- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh index 13ddcaf9a5b..5555667db44 100755 --- a/ci/ci-tests.sh +++ b/ci/ci-tests.sh @@ -77,14 +77,21 @@ grep '^max_level_' lightning/Cargo.toml | awk '{ print $1 }'| while read -r FEAT RUSTFLAGS="$RUSTFLAGS -A unused_variables -A unused_macros -A unused_imports -A dead_code" cargo check -p lightning --verbose --color always --features "$FEATURE" done +echo -e "\n\nTesting no-std builds" +for DIR in lightning-invoice; do + cargo test -p $DIR --verbose --color always --no-default-features + # 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 +done + echo -e "\n\nTesting no-std flags in various combinations" -for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do +for DIR in lightning 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 done -for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do +for DIR in lightning 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 done @@ -94,7 +101,7 @@ RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test --verbose --color always cargo test -p lightning --verbose --color always --no-default-features --features=std,_test_vectors # This one only works for lightning-invoice # check that compile with no-std and serde works in lightning-invoice -cargo test -p lightning-invoice --verbose --color always --no-default-features --features no-std --features serde +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-invoice/Cargo.toml b/lightning-invoice/Cargo.toml index f03875d67cd..2a5bc406b33 100644 --- 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 index bbb8dc8d5da..58f08583ad2 100644 --- 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/Cargo.toml b/lightning/Cargo.toml index 6fab8d5ecf3..d642b6fbd80 100644 --- a/lightning/Cargo.toml +++ b/lightning/Cargo.toml @@ -31,7 +31,7 @@ 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"] +no-std = ["hashbrown", "possiblyrandom", "libm"] std = ["lightning-invoice/std", "bech32/std"] # Generates low-r bitcoin signatures, which saves 1 byte in 50% of the cases diff --git a/no-std-check/Cargo.toml b/no-std-check/Cargo.toml index c9d404c922f..056cb1d2038 100644 --- 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", "lightning-rapid-gossip-sync/no-std"] [dependencies] lightning = { path = "../lightning", default-features = false } From 51f5bc881f051fa4c01fe086181bf25a5d3ae4b0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 16 Aug 2024 20:32:58 +0000 Subject: [PATCH 6/9] Drop the `no-std` feature from `lightning-rapid-gossip-sync` Now that we don't have to have everything in our entire ecosystem use the same `std`/`no-std` feature combinations we should start by untangling our own features a bit. This takes another step by removing the `no-std` feature entirely from the `lightning-rapid-gossip-sync` crate and removing all feature implications on dependencies from the remaining `std` feature. --- ci/ci-tests.sh | 6 +++--- lightning-rapid-gossip-sync/Cargo.toml | 3 +-- lightning-rapid-gossip-sync/src/lib.rs | 4 ++-- no-std-check/Cargo.toml | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh index 5555667db44..d41c86310b0 100755 --- a/ci/ci-tests.sh +++ b/ci/ci-tests.sh @@ -78,20 +78,20 @@ grep '^max_level_' lightning/Cargo.toml | awk '{ print $1 }'| while read -r FEAT done echo -e "\n\nTesting no-std builds" -for DIR in lightning-invoice; do +for DIR in lightning-invoice lightning-rapid-gossip-sync; do cargo test -p $DIR --verbose --color always --no-default-features # 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 done echo -e "\n\nTesting no-std flags in various combinations" -for DIR in lightning lightning-rapid-gossip-sync; do +for DIR in lightning; 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 done -for DIR in lightning lightning-rapid-gossip-sync; do +for DIR in lightning; 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 done diff --git a/lightning-rapid-gossip-sync/Cargo.toml b/lightning-rapid-gossip-sync/Cargo.toml index 9d9f3d23eb0..425c2c97d9d 100644 --- 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 index 04b564f04c0..2bae65cb8e8 100644 --- 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 { @@ -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/no-std-check/Cargo.toml b/no-std-check/Cargo.toml index 056cb1d2038..a94939eab48 100644 --- 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-rapid-gossip-sync/no-std"] +default = ["lightning/no-std"] [dependencies] lightning = { path = "../lightning", default-features = false } From e212d7429803833f01c98284a3607dcf58bcc134 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 17 Aug 2024 20:18:31 +0000 Subject: [PATCH 7/9] Somewhat clean up `ci-tests.sh` and sort by `RUSTFLAGS` --- ci/ci-tests.sh | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh index d41c86310b0..d6c8f051744 100755 --- a/ci/ci-tests.sh +++ b/ci/ci-tests.sh @@ -80,23 +80,26 @@ done 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 - # 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 done -echo -e "\n\nTesting no-std flags in various combinations" -for DIR in lightning; 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 -done +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; do +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 cargo test -p lightning --verbose --color always --no-default-features --features=std,_test_vectors # This one only works for lightning-invoice From 3fe4ef9640d83309488ef15bc9577c406a36856e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 17 Aug 2024 21:06:52 +0000 Subject: [PATCH 8/9] Drop the `_test_vectors` feature in favor of a `cfg` flag This exists just for tests, so there's no reason for it to be publicly visible. --- Cargo.toml | 1 + ci/ci-tests.sh | 2 +- lightning/Cargo.toml | 2 -- lightning/src/crypto/utils.rs | 4 ++-- lightning/src/ln/channel.rs | 7 +++++-- lightning/src/ln/channelmanager.rs | 4 ++-- lightning/src/ln/monitor_tests.rs | 4 ++-- 7 files changed, 13 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d1e2f0e054a..ae5d1bc8f0c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,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 index d6c8f051744..22d0783a97f 100755 --- a/ci/ci-tests.sh +++ b/ci/ci-tests.sh @@ -101,7 +101,7 @@ RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test -p lightning --verbose --colo echo -e "\n\nTesting other crate-specific builds" # Note that outbound_commitment_test only runs in this mode because of hardcoded signature values -cargo test -p lightning --verbose --color always --no-default-features --features=std,_test_vectors +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 # check that compile with no-std and serde works in lightning-invoice cargo test -p lightning-invoice --verbose --color always --no-default-features --features serde diff --git a/lightning/Cargo.toml b/lightning/Cargo.toml index d642b6fbd80..b8ebfdca210 100644 --- a/lightning/Cargo.toml +++ b/lightning/Cargo.toml @@ -28,8 +28,6 @@ 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", "libm"] std = ["lightning-invoice/std", "bech32/std"] diff --git a/lightning/src/crypto/utils.rs b/lightning/src/crypto/utils.rs index 98963c7c2bd..aab406157f8 100644 --- a/lightning/src/crypto/utils.rs +++ b/lightning/src/crypto/utils.rs @@ -75,9 +75,9 @@ pub fn sign_with_aux_rand( 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/channel.rs b/lightning/src/ln/channel.rs index 907a5058766..226fb50826d 100644 --- 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, hex: &str) -> PublicKey { + assert!(cfg!(not(feature = "grind_signatures"))); PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&>::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 index b0bf7611119..2684bb0f3a2 100644 --- 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 index 16f3b49262e..3fbcef3449b 100644 --- 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); From 8049f99ebccc9487b989eb670d88a4e57b9241eb Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 17 Aug 2024 21:13:08 +0000 Subject: [PATCH 9/9] Remove the `std` feature implications from the `lightning` crate Now that we don't have to have everything in our entire ecosystem use the same `std`/`no-std` feature combinations we should start by untangling our own features a bit. This takes one last step, removing the implications of the `std` feature from the `lightning` crate. --- lightning/Cargo.toml | 2 +- lightning/src/ln/bolt11_payment.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lightning/Cargo.toml b/lightning/Cargo.toml index b8ebfdca210..d68b8c6433d 100644 --- a/lightning/Cargo.toml +++ b/lightning/Cargo.toml @@ -30,7 +30,7 @@ max_level_trace = [] unsafe_revoked_tx_signing = [] no-std = ["hashbrown", "possiblyrandom", "libm"] -std = ["lightning-invoice/std", "bech32/std"] +std = [] # Generates low-r bitcoin signatures, which saves 1 byte in 50% of the cases grind_signatures = [] diff --git a/lightning/src/ln/bolt11_payment.rs b/lightning/src/ln/bolt11_payment.rs index 596ee49aca1..b4863d4b1e1 100644 --- 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())