lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.16k stars 367 forks source link

Fix builds of `lightning-rapid-gossip-sync` and `lightning-background-processor` crates #3289

Closed tnull closed 2 months ago

tnull commented 2 months ago

Previously, we would only check the builds for the workspace as a whole. This however would mean that we would check/test crates with lightning's default features enabled, allowing failures-to-build under the crates own default features to slip through, if they didn't explicitly enablelightning/std, for example.

Here, we fix the regressions in the builds of the lightning-rapid-gossip-sync and lightning-background-processor crates, and extend the CI to run checks, tests, and doc generation on the workspace members individually, asserting that all of them build even when not built as part of the same workspace as lightning.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.91%. Comparing base (6662c5c) to head (545b037). Report is 29 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3289 +/- ## ========================================== + Coverage 89.65% 89.91% +0.26% ========================================== Files 126 126 Lines 102546 104476 +1930 Branches 102546 104476 +1930 ========================================== + Hits 91935 93941 +2006 + Misses 7890 7841 -49 + Partials 2721 2694 -27 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tnull commented 2 months ago

As discussed elsewhere I now added a commit dropping lightning's no-std feature and also added a fixup so that RGS no only depends on bitcoin-io/std (which also requires bitcoin_hashes/std).

TheBlueMatt commented 2 months ago

CI is very sad :/

tnull commented 2 months ago

Now added a commit fixing doc generation for lightning without std set. Fingers crossed this improves my standing with said CI.

TheBlueMatt commented 2 months ago

Feel free to squash imo.

tnull commented 2 months ago

Feel free to squash imo.

Squashed without further changes.

TheBlueMatt commented 2 months ago

Oops, looks like fuzzing CI is still trying to pass a no-std feature.

tnull commented 2 months ago

Oops, looks like fuzzing CI is still trying to pass a no-std feature.

Ah, actually not fuzzing, but the check-compiles script.

Amended and force-pushed with the following changes:

diff --git a/ci/check-compiles.sh b/ci/check-compiles.sh
index 7ad9f4df1..a067861fb 100755
--- a/ci/check-compiles.sh
+++ b/ci/check-compiles.sh
@@ -7,4 +7,4 @@ cargo doc
 cargo doc --document-private-items
 cd fuzz && RUSTFLAGS="--cfg=fuzzing --cfg=secp256k1_fuzz --cfg=hashes_fuzz" cargo check --features=stdin_fuzz
-cd ../lightning && cargo check --no-default-features --features=no-std
+cd ../lightning && cargo check --no-default-features
 cd .. && RUSTC_BOOTSTRAP=1 RUSTFLAGS="--cfg=c_bindings" cargo check -Z avoid-dev-deps
diff --git a/lightning-background-processor/Cargo.toml b/lightning-background-processor/Cargo.toml
index d73499fd4..0afc18fdf 100644
--- a/lightning-background-processor/Cargo.toml
+++ b/lightning-background-processor/Cargo.toml
@@ -16,5 +16,5 @@ rustdoc-args = ["--cfg", "docsrs"]
 [features]
 futures = [ ]
-std = ["lightning/std", "lightning-rapid-gossip-sync/std"]
+std = ["lightning/std", "bitcoin-io/std", "bitcoin_hashes/std"]

 default = ["std"]
@@ -22,4 +22,6 @@ default = ["std"]
 [dependencies]
 bitcoin = { version = "0.32.2", default-features = false }
+bitcoin_hashes = { version = "0.14.0", default-features = false }
+bitcoin-io = { version = "0.1.2", default-features = false }
 lightning = { version = "0.0.124", path = "../lightning", default-features = false }
 lightning-rapid-gossip-sync = { version = "0.0.124", path = "../lightning-rapid-gossip-sync", default-features = false }