From bd8bdb4b0c7f0c91af1ac1f45e6b9550d2ce46a4 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 29 Jan 2025 11:42:20 +1100 Subject: [PATCH] Remove default feature The default feature in node overides an explicit version feature. This is a footgun. Currently the logic around using `bitcoind` from the environment is borked. Also, features in the `node` crate are unusual in that at least one version feature is expected. Lets make this a build requirement. Maintain the current behaviour in relation to `--all-features` i.e., all features implies latest version 28.0 - Remove `default` feature - Remove `download` from version number features - Use `compile_error` to catch missing version feature --- integration_test/Cargo.toml | 2 +- node/Cargo.toml | 49 +++++++++++++++++++------------------ node/src/client_versions.rs | 6 +---- node/src/versions.rs | 43 ++++++++++++++++---------------- 4 files changed, 49 insertions(+), 51 deletions(-) diff --git a/integration_test/Cargo.toml b/integration_test/Cargo.toml index 461b289..6552683 100644 --- a/integration_test/Cargo.toml +++ b/integration_test/Cargo.toml @@ -51,7 +51,7 @@ TODO = [] # This is a dirty hack while writing the tests. [dependencies] bitcoin = { version = "0.32.0", default-features = false, features = ["std", "serde"] } client = { package = "corepc-client", version = "0.5.0", default-features = false, features = ["client-sync"] } -node = { package = "corepc-node", version = "0.5.0", default-features = false, features = [] } +node = { package = "corepc-node", version = "0.5.0", default-features = false, features = ["download"] } rand = "0.8.5" env_logger = "0.9.0" diff --git a/node/Cargo.toml b/node/Cargo.toml index c2c0c96..f5d1e17 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -35,36 +35,37 @@ anyhow = "1.0.66" # Please note, in this crate the version features are mutally exclusive. # -# - `cargo test --all-features` is the same as `cargo test --features=v28_0` -# - To use --no-default-features you must configure the bitcoind binary to be version 28 either by having `bitcoind` in your path or using `BITCOIND_EXE=/path/to/bitcoind`. -# - `cargo test --no-default-features` uses `v28_0` also. +# - `cargo test --features=27_2,download` to download Bitcoin Core binary v27. +# - `cargo test --all-features` is the same as `cargo test --features=download,v28_0` +# - `cargo test --features=28_0` to use `bitcoind`. +# +# If you do not enable `download` then you must configure the bitcoind binary by `bitcoind` in your +# path or using `BITCOIND_EXE=/path/to/bitcoind`. Note also that the feature enabled should match +# the version from `bitcoind --version`. [features] -default = ["28_0"] - -# download is not supposed to be used directly only through selecting one of the version feature download = ["bitcoin_hashes", "flate2", "tar", "minreq", "zip"] # We support all minor releases of the latest three versions. -28_0 = ["download", "27_2"] -27_2 = ["download", "27_1"] -27_1 = ["download", "27_0"] -27_0 = ["download", "26_2"] -26_2 = ["download", "26_1"] -26_1 = ["download", "26_0"] -26_0 = ["download", "25_2"] -# We only support the latest minor version for older versions. -25_2 = ["download", "24_2"] -24_2 = ["download", "23_2"] -23_2 = ["download", "22_1"] -22_1 = ["download", "0_21_2"] -0_21_2 = ["download", "0_20_2"] -0_20_2 = ["download", "0_19_1"] -0_19_1 = ["download", "0_18_1"] -0_18_1 = ["download", "0_17_1"] -0_17_1 = ["download"] +28_0 = ["27_2"] +27_2 = ["27_1"] +27_1 = ["27_0"] +27_0 = ["26_2"] +26_2 = ["26_1"] +26_1 = ["26_0"] +26_0 = ["25_2"] -doc = [] # used only for documentation building +# We only support the latest minor version for older versions. +25_2 = ["24_2"] +24_2 = ["23_2"] +23_2 = ["22_1"] +22_1 = ["0_21_2"] +0_21_2 = ["0_20_2"] +0_20_2 = ["0_19_1"] +0_19_1 = ["0_18_1"] +0_18_1 = ["0_17_1"] +0_17_1 = [] +doc = [] # Used only for documentation building. [package.metadata.docs.rs] features = ["download", "doc", "28_0"] diff --git a/node/src/client_versions.rs b/node/src/client_versions.rs index 3f9c507..4f23e28 100644 --- a/node/src/client_versions.rs +++ b/node/src/client_versions.rs @@ -1,6 +1,6 @@ // The version specific client and json types. // -// **THIS IS AVAILABLE FOR ALL VERSION NUMBER FEATURES** (eg `25_0`, `24_2` etc). This crate is +// **THIS IS AVAILABLE FOR ALL VERSION NUMBER FEATURES** (eg `25_2`, `28_0` etc). This crate is // unusual in that it expects exactly one version number feature to be selected, docs.rs is not set // up to handle such oddity. @@ -53,7 +53,3 @@ pub use corepc_client::{client_sync::v18::{Client, AddressType}, types::v18 as t #[cfg(all(feature = "0_17_1", not(feature = "0_18_1")))] pub use corepc_client::{client_sync::v17::{Client, AddressType}, types::v17 as types}; - -// To make --no-default-features work we have to re-export a the types, use most recent version same as we do for all features. -#[cfg(all(not(feature = "28_0"), not(feature = "27_1"), not(feature = "27_0"), not(feature = "26_2"), not(feature = "26_1"), not(feature = "26_0"), not(feature = "25_2"), not(feature = "24_2"), not(feature = "23_2"), not(feature = "22_1"), not(feature = "0_21_2"), not(feature = "0_20_2"), not(feature = "0_19_1"), not(feature = "0_18_1"), not(feature = "0_17_1")))] -pub use corepc_client::{client_sync::v28::{Client, AddressType}, types::v28 as types}; diff --git a/node/src/versions.rs b/node/src/versions.rs index 3af085d..047796c 100644 --- a/node/src/versions.rs +++ b/node/src/versions.rs @@ -1,3 +1,25 @@ +// An explicit version of Bitcoin Core must be selected by enabling some feature. +// We check this here instead of in `lib.rs` because this file is included in `build.rs`. +#[cfg(all( + not(feature = "28_0"), + not(feature = "27_2"), + not(feature = "27_1"), + not(feature = "27_0"), + not(feature = "26_2"), + not(feature = "26_1"), + not(feature = "26_0"), + not(feature = "25_2"), + not(feature = "24_2"), + not(feature = "23_2"), + not(feature = "22_1"), + not(feature = "0_21_2"), + not(feature = "0_20_2"), + not(feature = "0_19_1"), + not(feature = "0_18_1"), + not(feature = "0_17_1") +))] +compile_error!("enable a feature in order to select the version of Bitcoin Core to use"); + #[cfg(feature = "28_0")] pub const VERSION: &str = "28.0"; @@ -45,24 +67,3 @@ pub const VERSION: &str = "0.18.1"; #[cfg(all(feature = "0_17_1", not(feature = "0_18_1")))] pub const VERSION: &str = "0.17.1"; - -// To make --no-default-features work we have to enable some feature, use most recent version same as for default. -#[cfg(all( - not(feature = "28_0"), - not(feature = "27_1"), - not(feature = "27_0"), - not(feature = "26_2"), - not(feature = "26_1"), - not(feature = "26_0"), - not(feature = "25_2"), - not(feature = "24_2"), - not(feature = "23_2"), - not(feature = "22_1"), - not(feature = "0_21_2"), - not(feature = "0_20_2"), - not(feature = "0_19_1"), - not(feature = "0_18_1"), - not(feature = "0_17_1") -))] -#[allow(dead_code)] // for --no-default-features -pub const VERSION: &str = "28.0";