Skip to content

Commit

Permalink
Remove default feature
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tcharding committed Jan 29, 2025
1 parent 8054514 commit bd8bdb4
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 51 deletions.
2 changes: 1 addition & 1 deletion integration_test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
49 changes: 25 additions & 24 deletions node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
6 changes: 1 addition & 5 deletions node/src/client_versions.rs
Original file line number Diff line number Diff line change
@@ -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.

Expand Down Expand Up @@ -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};
43 changes: 22 additions & 21 deletions node/src/versions.rs
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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";

0 comments on commit bd8bdb4

Please sign in to comment.