Merge rust-bitcoin#45: node: Remove default feature

bd8bdb4 Remove default feature (Tobin C. Harding)
8054514 node: Allow unused_imports module wide (Tobin C. Harding)

Pull request description:

  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

  Fix: rust-bitcoin#40

ACKs for top commit:
  apoelstra:
    ACK bd8bdb4; successfully ran local tests; thanks, I think this is the right approach

Tree-SHA512: 5b62b3fbd6b77eeed7f8830e6aedabdf0281e5b1f171df3bab55986a2c3860c8bfb97d21619c3c2c4f10c5e369e996840c3f7e3890f943330a930a7476f96826

Author: tcharding

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"
 

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"]

node/src/client_versions.rs

@@ -1,74 +1,55 @@
 // 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.
 
+#![allow(unused_imports)] // Not all users need the json types.
+
 #[cfg(feature = "28_0")]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v28::{Client, AddressType}, types::v28 as types};
 
 #[cfg(all(feature = "27_2", not(feature = "28_0")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v27::{Client, AddressType}, types::v27 as types};
 
 #[cfg(all(feature = "27_1", not(feature = "27_2")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v27::{Client, AddressType}, types::v27 as types};
 
 #[cfg(all(feature = "27_0", not(feature = "27_1")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v27::{Client, AddressType}, types::v27 as types};
 
 #[cfg(all(feature = "26_2", not(feature = "27_0")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v26::{Client, AddressType}, types::v26 as types};
 
 #[cfg(all(feature = "26_1", not(feature = "26_2")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v26::{Client, AddressType}, types::v26 as types};
 
 #[cfg(all(feature = "26_0", not(feature = "26_1")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v26::{Client, AddressType}, types::v26 as types};
 
 #[cfg(all(feature = "25_2", not(feature = "26_0")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v25::{Client, AddressType}, types::v25 as types};
 
 #[cfg(all(feature = "24_2", not(feature = "25_2")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v24::{Client, AddressType}, types::v24 as types};
 
 #[cfg(all(feature = "23_2", not(feature = "24_2")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v23::{Client, AddressType}, types::v23 as types};
 
 #[cfg(all(feature = "22_1", not(feature = "23_2")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v22::{Client, AddressType}, types::v22 as types};
 
 #[cfg(all(feature = "0_21_2", not(feature = "22_1")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v21::{Client, AddressType}, types::v21 as types};
 
 #[cfg(all(feature = "0_20_2", not(feature = "0_21_2")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v20::{Client, AddressType}, types::v20 as types};
 
 #[cfg(all(feature = "0_19_1", not(feature = "0_20_2")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v19::{Client, AddressType}, types::v19 as types};
 
 #[cfg(all(feature = "0_18_1", not(feature = "0_19_1")))]
-#[allow(unused_imports)] // Not all users need the json types.
 pub use corepc_client::{client_sync::v18::{Client, AddressType}, types::v18 as types};
 
 #[cfg(all(feature = "0_17_1", not(feature = "0_18_1")))]
-#[allow(unused_imports)] // Not all users need the json types.
 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")))]
-#[allow(unused_imports)] // Not all users need the json types.
-pub use corepc_client::{client_sync::v28::{Client, AddressType}, types::v28 as types};

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";