From c197b2142826cb346289f2f2abc2f4e2365ca456 Mon Sep 17 00:00:00 2001 From: Mathis Date: Mon, 18 Nov 2024 15:55:33 +0100 Subject: [PATCH] Use espresso-types without testing feature (#2283) * Use espresso-types without testing feature We would like to be able to use the types crate in other crates but currently it won't compile without the testing feature enabled. This PR fixes that. - Use `NullStateCatchup` instead of `MockStateCatchup` where we don't need catchup. - Feature gate `MockStateCatchup` behind testing in types crate. - Remove the unused `InstanceState` argument from `from_transactions_sync`. * Clean up features in sequencer crate - `testing` feature enables `testing` feature of types and utils crate. - Running tests enables `testing` feature of crate via dev-dependencies. This allows to run `cargo check` and `cargo check --tests` in the sequencer crate. Previously `cargo check` was broken without the `testing` feature enabled explicitly. * WIP: remove testing feature from non test code * Update query service * refactor: remove libp2p feature The libp2p feature was always turned on and the code didn't compile without it. With this change all feature sets checked with `cargo hack check --each-feature` compile successfully. * ci: check each cargo feature * check compilation of each feature with --tests * CI: run cargo check for feature combinations --- .github/workflows/cargo-features.yml | 42 +++++++++++++++++++ Cargo.lock | 6 ++- Cargo.toml | 2 +- builder/Cargo.toml | 8 +--- client/Cargo.toml | 2 +- flake.nix | 3 +- hotshot-state-prover/Cargo.toml | 2 +- justfile | 3 ++ marketplace-builder/Cargo.toml | 7 ++-- marketplace-solver/Cargo.toml | 6 +-- node-metrics/Cargo.toml | 3 ++ sequencer/Cargo.toml | 20 ++++++--- sequencer/src/api.rs | 17 ++++---- sequencer/src/lib.rs | 25 +++-------- sequencer/src/network/mod.rs | 4 -- types/Cargo.toml | 5 +-- .../v0/impls/block/full_payload/payload.rs | 9 +--- types/src/v0/impls/mod.rs | 5 ++- types/src/v0/mod.rs | 7 +++- 19 files changed, 106 insertions(+), 70 deletions(-) create mode 100644 .github/workflows/cargo-features.yml diff --git a/.github/workflows/cargo-features.yml b/.github/workflows/cargo-features.yml new file mode 100644 index 0000000000..262f678fc1 --- /dev/null +++ b/.github/workflows/cargo-features.yml @@ -0,0 +1,42 @@ +name: Cargo features + +on: + push: + branches: + - main + - release-* + tags: + # YYYYMMDD + - "20[0-9][0-9][0-1][0-9][0-3][0-9]*" + schedule: + - cron: "0 0 * * 1" + pull_request: + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + cargo-features: + runs-on: ubuntu-latest + steps: + - uses: taiki-e/install-action@cargo-hack + + - name: Checkout Repository + uses: actions/checkout@v4 + + # Note: this job doesn't use a cache on purpose because it mostly compiles + # the crates in this repo over and over again with different feature + # combinations. Adding caching would not speed it up much and further + # contribute to our cache usage. + + # Includes checks for `--no-default-features` and `--all-features` as well + # as each individual feature enabled. + - name: Check compilation for feature combinations + run: | + cargo hack check --feature-powerset + + - name: Check compilation for feature combinations (--tests) + run: | + cargo hack check --feature-powerset --tests diff --git a/Cargo.lock b/Cargo.lock index 88964103ce..14d6c473c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2860,7 +2860,6 @@ dependencies = [ "hotshot", "hotshot-orchestrator", "hotshot-query-service", - "hotshot-testing", "hotshot-types", "itertools 0.12.1", "jf-merkle-tree", @@ -4242,7 +4241,7 @@ dependencies = [ [[package]] name = "hotshot-query-service" version = "0.1.62" -source = "git+https://github.com/EspressoSystems/hotshot-query-service?tag=0.1.69#1335419bd0a451fe129185c849c8790f26c59547" +source = "git+https://github.com/EspressoSystems/hotshot-query-service?tag=0.1.70#b41a6307db861802c0c88ab216ae01ebd9e5a699" dependencies = [ "anyhow", "ark-serialize", @@ -6330,6 +6329,7 @@ dependencies = [ "hotshot-query-service", "hotshot-types", "jf-signature 0.2.0", + "marketplace-solver", "portpicker", "rand 0.8.5", "serde", @@ -6684,6 +6684,7 @@ dependencies = [ "hotshot-query-service", "hotshot-stake-table", "hotshot-types", + "node-metrics", "prometheus-parse", "reqwest 0.12.9", "serde", @@ -8705,6 +8706,7 @@ dependencies = [ "rand_chacha 0.3.1", "rand_distr", "reqwest 0.12.9", + "sequencer", "sequencer-utils", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 211840c92a..9bf093c050 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,7 +68,7 @@ marketplace-builder-core = { git = "https://github.com/EspressoSystems/marketpla marketplace-builder-shared = { git = "https://github.com/EspressoSystems/marketplace-builder-core", tag = "0.1.56" } hotshot-events-service = { git = "https://github.com/EspressoSystems/hotshot-events-service.git", tag = "0.1.54" } hotshot-orchestrator = { git = "https://github.com/EspressoSystems/hotshot", tag = "0.5.81" } -hotshot-query-service = { git = "https://github.com/EspressoSystems/hotshot-query-service", tag = "0.1.69" } +hotshot-query-service = { git = "https://github.com/EspressoSystems/hotshot-query-service", tag = "0.1.70" } hotshot-stake-table = { git = "https://github.com/EspressoSystems/hotshot", tag = "0.5.81" } hotshot-state-prover = { version = "0.1.0", path = "hotshot-state-prover" } hotshot-task = { git = "https://github.com/EspressoSystems/hotshot", tag = "0.5.81" } diff --git a/builder/Cargo.toml b/builder/Cargo.toml index 893396302d..d23387f874 100644 --- a/builder/Cargo.toml +++ b/builder/Cargo.toml @@ -5,10 +5,6 @@ version = { workspace = true } authors = { workspace = true } edition = { workspace = true } -[features] -default = ["libp2p"] -libp2p = ["sequencer/libp2p"] - [dependencies] anyhow = { workspace = true } async-broadcast = { workspace = true } @@ -17,7 +13,7 @@ async-trait = { workspace = true } clap = { workspace = true } committable = { workspace = true } dotenvy = { workspace = true } -espresso-types = { path = "../types", features = ["testing"] } +espresso-types = { path = "../types" } ethers = { workspace = true } futures = { workspace = true } hotshot = { workspace = true } @@ -34,7 +30,7 @@ libp2p-networking = { workspace = true } marketplace-builder-shared = { workspace = true } portpicker = { workspace = true } rand = "0.8.5" -sequencer = { path = "../sequencer", features = ["testing"] } +sequencer = { path = "../sequencer" } sequencer-utils = { path = "../utils" } serde = { workspace = true } surf = "2.3.1" diff --git a/client/Cargo.toml b/client/Cargo.toml index 103f8a5090..94c5e7d819 100644 --- a/client/Cargo.toml +++ b/client/Cargo.toml @@ -8,7 +8,7 @@ publish = false [dependencies] anyhow = { workspace = true } contract-bindings = { path = "../contract-bindings" } -espresso-types = { path = "../types", features = ["testing"] } +espresso-types = { path = "../types" } ethers = { workspace = true } futures = { workspace = true } jf-merkle-tree = { workspace = true } diff --git a/flake.nix b/flake.nix index f4a4368fa7..80e45363fe 100644 --- a/flake.nix +++ b/flake.nix @@ -192,8 +192,9 @@ # Rust tools cargo-audit cargo-edit - cargo-sort + cargo-hack cargo-nextest + cargo-sort typos just nightlyToolchain.passthru.availableComponents.rust-analyzer diff --git a/hotshot-state-prover/Cargo.toml b/hotshot-state-prover/Cargo.toml index aad8a95c9a..b0647dff0a 100644 --- a/hotshot-state-prover/Cargo.toml +++ b/hotshot-state-prover/Cargo.toml @@ -16,7 +16,7 @@ ark-std = { workspace = true } clap = { workspace = true } contract-bindings = { path = "../contract-bindings" } displaydoc = { version = "0.2.3", default-features = false } -espresso-types = { path = "../types", features = ["testing"] } +espresso-types = { path = "../types" } ethers = { workspace = true } futures = { workspace = true } hotshot-contract-adapter = { workspace = true } diff --git a/justfile b/justfile index 1f8512f7d3..1bfd6f3eeb 100644 --- a/justfile +++ b/justfile @@ -55,6 +55,9 @@ test-integration: clippy: cargo clippy --workspace --all-features --all-targets -- -D warnings +check-features *args: + cargo hack check --each-feature {{args}} + # Helpful shortcuts for local development dev-orchestrator: target/release/orchestrator -p 8080 -n 1 diff --git a/marketplace-builder/Cargo.toml b/marketplace-builder/Cargo.toml index e7734d1017..90291b6e8f 100644 --- a/marketplace-builder/Cargo.toml +++ b/marketplace-builder/Cargo.toml @@ -6,8 +6,6 @@ authors = { workspace = true } edition = { workspace = true } [features] -default = ["libp2p"] -libp2p = ["sequencer/libp2p"] testing = ["hotshot-query-service", "sequencer-utils", "tempfile"] [dependencies] @@ -31,9 +29,9 @@ jf-merkle-tree = { workspace = true } jf-signature = { workspace = true, features = ["bls"] } marketplace-builder-core = { workspace = true } marketplace-builder-shared = { workspace = true } -marketplace-solver = { path = "../marketplace-solver", features = ["testing"] } +marketplace-solver = { path = "../marketplace-solver" } portpicker = { workspace = true } -sequencer = { path = "../sequencer", features = ["testing"] } +sequencer = { path = "../sequencer" } sequencer-utils = { path = "../utils", optional = true } surf = "2.3.1" surf-disco = { workspace = true } @@ -46,6 +44,7 @@ vbs = { workspace = true } [dev-dependencies] hotshot-query-service = { workspace = true } +marketplace-solver = { path = "../marketplace-solver", features = ["testing"] } sequencer = { path = "../sequencer", features = ["testing"] } sequencer-utils = { path = "../utils", features = ["testing"] } tempfile = { workspace = true } diff --git a/marketplace-solver/Cargo.toml b/marketplace-solver/Cargo.toml index 3cb8b5ff32..b6fdbfbf91 100644 --- a/marketplace-solver/Cargo.toml +++ b/marketplace-solver/Cargo.toml @@ -6,9 +6,8 @@ edition = "2021" [features] testing = [ - "hotshot-query-service", + "hotshot-query-service/testing", "portpicker", - ] [dependencies] @@ -19,7 +18,7 @@ bincode = { workspace = true } clap = { workspace = true } cld = { workspace = true } committable = { workspace = true } -espresso-types = { path = "../types", features = [ "testing" ] } +espresso-types = { path = "../types" } futures = { workspace = true } hotshot = { workspace = true } hotshot-events-service = { workspace = true } @@ -40,4 +39,5 @@ tracing = { workspace = true } vbs = { workspace = true } [dev-dependencies] +marketplace-solver = { path = ".", features = [ "testing" ] } portpicker = { workspace = true } diff --git a/node-metrics/Cargo.toml b/node-metrics/Cargo.toml index f57d5f4b73..eb441ca658 100644 --- a/node-metrics/Cargo.toml +++ b/node-metrics/Cargo.toml @@ -8,6 +8,9 @@ edition = { workspace = true } [features] testing = ["serde_json", "espresso-types/testing"] +[dev-dependencies] +node-metrics = { path = ".", features = [ "testing" ] } + [dependencies] async-lock = { workspace = true } async-trait = { workspace = true } diff --git a/sequencer/Cargo.toml b/sequencer/Cargo.toml index 0511356264..eb1789656c 100644 --- a/sequencer/Cargo.toml +++ b/sequencer/Cargo.toml @@ -6,9 +6,15 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [features] -default = ["libp2p"] -testing = ["hotshot-testing", "marketplace-builder-core", "marketplace-builder-shared", "hotshot-builder-api"] -libp2p = [] +testing = [ + "hotshot-testing", + "marketplace-builder-core", + "marketplace-builder-shared", + "hotshot-builder-api", + "espresso-types/testing", + "sequencer-utils/testing", + "hotshot-query-service/testing", +] benchmarking = [] [[bin]] @@ -19,13 +25,16 @@ required-features = ["testing"] escargot = "0.5.10" espresso-macros = { git = "https://github.com/EspressoSystems/espresso-macros.git", tag = "0.1.0" } hotshot-example-types = { workspace = true } -hotshot-query-service = { workspace = true, features = ["testing"] } +hotshot-query-service = { workspace = true } hotshot-testing = { workspace = true } pretty_assertions = { workspace = true } rand = "0.8.5" reqwest = { workspace = true } tempfile = { workspace = true } +# Enable "testing" feature when running tests +sequencer = { path = ".", features = [ "testing" ] } + [build-dependencies] anyhow = { workspace = true } vergen = { workspace = true } @@ -54,7 +63,7 @@ derivative = "2.2" derive_more = { workspace = true } dotenvy = { workspace = true } dyn-clone = { workspace = true } -espresso-types = { path = "../types", features = ["testing"] } +espresso-types = { path = "../types" } ethers = { workspace = true } futures = { workspace = true } @@ -115,6 +124,7 @@ tracing-subscriber = "0.3.18" url = { workspace = true } vbs = { workspace = true } vec1 = { workspace = true } + [package.metadata.cargo-udeps.ignore] normal = ["hotshot-testing"] diff --git a/sequencer/src/api.rs b/sequencer/src/api.rs index 8faecc8ff6..e89c635507 100644 --- a/sequencer/src/api.rs +++ b/sequencer/src/api.rs @@ -469,7 +469,6 @@ pub mod test_helpers { use tokio::{spawn, time::sleep}; use espresso_types::{ - mock::MockStateCatchup, v0::traits::{NullEventConsumer, PersistenceOptions, StateCatchup}, MarketplaceVersion, NamespaceId, ValidatedState, }; @@ -497,6 +496,7 @@ pub mod test_helpers { use super::*; use crate::{ + catchup::NullStateCatchup, persistence::no_storage, testing::{ run_marketplace_builder, run_test_builder, wait_for_decide_on_handle, TestConfig, @@ -536,12 +536,12 @@ pub mod test_helpers { network_config: Option>, } - impl Default for TestNetworkConfigBuilder<5, no_storage::Options, MockStateCatchup> { + impl Default for TestNetworkConfigBuilder<5, no_storage::Options, NullStateCatchup> { fn default() -> Self { TestNetworkConfigBuilder { state: std::array::from_fn(|_| ValidatedState::default()), persistence: Some([no_storage::Options; 5]), - catchup: Some(std::array::from_fn(|_| MockStateCatchup::default())), + catchup: Some(std::array::from_fn(|_| NullStateCatchup::default())), network_config: None, api_config: None, } @@ -549,15 +549,15 @@ pub mod test_helpers { } impl - TestNetworkConfigBuilder<{ NUM_NODES }, no_storage::Options, MockStateCatchup> + TestNetworkConfigBuilder<{ NUM_NODES }, no_storage::Options, NullStateCatchup> { pub fn with_num_nodes( - ) -> TestNetworkConfigBuilder<{ NUM_NODES }, no_storage::Options, MockStateCatchup> + ) -> TestNetworkConfigBuilder<{ NUM_NODES }, no_storage::Options, NullStateCatchup> { TestNetworkConfigBuilder { state: std::array::from_fn(|_| ValidatedState::default()), persistence: Some([no_storage::Options; { NUM_NODES }]), - catchup: Some(std::array::from_fn(|_| MockStateCatchup::default())), + catchup: Some(std::array::from_fn(|_| NullStateCatchup::default())), network_config: None, api_config: None, } @@ -1304,7 +1304,6 @@ mod test { use tokio::time::sleep; use espresso_types::{ - mock::MockStateCatchup, traits::NullEventConsumer, v0_1::{UpgradeMode, ViewBasedUpgrade}, BackoffParams, FeeAccount, FeeAmount, Header, MockSequencerVersions, SequencerVersions, @@ -1344,7 +1343,7 @@ mod test { }; use super::*; use crate::{ - catchup::StatePeers, + catchup::{NullStateCatchup, StatePeers}, persistence::no_storage, testing::{TestConfig, TestConfigBuilder}, }; @@ -1360,7 +1359,7 @@ mod test { let anvil = Anvil::new().spawn(); let l1 = anvil.endpoint().parse().unwrap(); let network_config = TestConfigBuilder::default().l1_url(l1).build(); - let config = TestNetworkConfigBuilder::<5, _, MockStateCatchup>::default() + let config = TestNetworkConfigBuilder::<5, _, NullStateCatchup>::default() .api_config(options) .network_config(network_config) .build(); diff --git a/sequencer/src/lib.rs b/sequencer/src/lib.rs index a4e7ed4fca..1eb06f4fca 100644 --- a/sequencer/src/lib.rs +++ b/sequencer/src/lib.rs @@ -18,7 +18,6 @@ use espresso_types::{ SolverAuctionResultsProvider, ValidatedState, }; use ethers::types::U256; -#[cfg(feature = "libp2p")] use futures::FutureExt; use genesis::L1Finalized; use hotshot::traits::election::static_committee::StaticCommittee; @@ -33,15 +32,9 @@ use tracing::info; use url::Url; pub mod persistence; pub mod state; - -#[cfg(feature = "libp2p")] -use std::time::Duration; -use std::{collections::BTreeMap, fmt::Debug, marker::PhantomData}; - use derivative::Derivative; use espresso_types::v0::traits::{PersistenceOptions, SequencerPersistence}; pub use genesis::Genesis; -#[cfg(feature = "libp2p")] use hotshot::traits::implementations::{ derive_libp2p_multiaddr, CombinedNetworks, GossipConfig, Libp2pNetwork, RequestResponseConfig, }; @@ -71,6 +64,8 @@ use hotshot_types::{ }; pub use options::Options; use serde::{Deserialize, Serialize}; +use std::time::Duration; +use std::{collections::BTreeMap, fmt::Debug, marker::PhantomData}; use vbs::version::{StaticVersion, StaticVersionType}; pub mod network; @@ -447,8 +442,7 @@ pub async fn init_node( response_size_maximum: network_params.libp2p_max_direct_transmit_size, }; - // Initialize the Libp2p network (if enabled) - #[cfg(feature = "libp2p")] + // Initialize the Libp2p network let network = { let p2p_network = Libp2pNetwork::from_config( network_config.clone(), @@ -488,15 +482,6 @@ pub async fn init_node( )) }; - // Wait for the CDN network to be ready if we're not using the P2P network - #[cfg(not(feature = "libp2p"))] - let network = { - tracing::warn!("Waiting for the CDN connection to be initialized"); - cdn_network.wait_for_ready().await; - tracing::warn!("CDN connection initialized"); - Arc::from(cdn_network) - }; - let mut genesis_state = ValidatedState { chain_config: genesis.chain_config.into(), ..Default::default() @@ -566,10 +551,10 @@ pub fn empty_builder_commitment() -> BuilderCommitment { pub mod testing { use std::{collections::HashMap, time::Duration}; + use catchup::NullStateCatchup; use committable::Committable; use espresso_types::{ eth_signature_key::EthKeyPair, - mock::MockStateCatchup, v0::traits::{EventConsumer, NullEventConsumer, PersistenceOptions, StateCatchup}, Event, FeeAccount, Leaf, MarketplaceVersion, Payload, PubKey, SeqTypes, Transaction, Upgrade, @@ -928,7 +913,7 @@ pub mod testing { i, ValidatedState::default(), no_storage::Options, - MockStateCatchup::default(), + NullStateCatchup::default(), &NoMetrics, STAKE_TABLE_CAPACITY_FOR_TEST, NullEventConsumer, diff --git a/sequencer/src/network/mod.rs b/sequencer/src/network/mod.rs index 57d666ac42..d16c35f999 100644 --- a/sequencer/src/network/mod.rs +++ b/sequencer/src/network/mod.rs @@ -5,10 +5,6 @@ use super::*; pub mod cdn; pub mod libp2p; -#[cfg(feature = "libp2p")] pub type Production = CombinedNetworks; -#[cfg(not(feature = "libp2p"))] -pub type Production = PushCdnNetwork; - pub type Memory = MemoryNetwork; diff --git a/types/Cargo.toml b/types/Cargo.toml index 35bb9593f5..f401a8a5f2 100644 --- a/types/Cargo.toml +++ b/types/Cargo.toml @@ -5,7 +5,7 @@ authors = ["Espresso Systems "] edition = "2021" [features] -testing = ["hotshot-testing"] +testing = [] [dependencies] @@ -28,8 +28,7 @@ fluent-asserter = "0.1.9" futures = { workspace = true } hotshot = { workspace = true } hotshot-orchestrator = { workspace = true } -hotshot-query-service = { workspace = true, features = ["testing"] } -hotshot-testing = { workspace = true, optional = true } +hotshot-query-service = { workspace = true } hotshot-types = { workspace = true } itertools = { workspace = true } jf-merkle-tree = { workspace = true } diff --git a/types/src/v0/impls/block/full_payload/payload.rs b/types/src/v0/impls/block/full_payload/payload.rs index 5948467837..eedebc9ba1 100644 --- a/types/src/v0/impls/block/full_payload/payload.rs +++ b/types/src/v0/impls/block/full_payload/payload.rs @@ -74,7 +74,6 @@ impl Payload { fn from_transactions_sync( transactions: impl IntoIterator>::Transaction> + Send, chain_config: ChainConfig, - _instance_state: &>::Instance, ) -> Result< (Self, >::Metadata), >::Error, @@ -158,11 +157,7 @@ impl BlockPayload for Payload { } }; - Self::from_transactions_sync( - transactions, - ChainConfig::from(chain_config), - instance_state, - ) + Self::from_transactions_sync(transactions, ChainConfig::from(chain_config)) } // TODO avoid cloning the entire payload here? @@ -174,7 +169,7 @@ impl BlockPayload for Payload { } fn empty() -> (Self, Self::Metadata) { - let payload = Self::from_transactions_sync(vec![], Default::default(), &Default::default()) + let payload = Self::from_transactions_sync(vec![], Default::default()) .unwrap() .0; diff --git a/types/src/v0/impls/mod.rs b/types/src/v0/impls/mod.rs index 69cdee5323..eb72ebb143 100644 --- a/types/src/v0/impls/mod.rs +++ b/types/src/v0/impls/mod.rs @@ -13,6 +13,9 @@ mod transaction; pub use auction::SolverAuctionResultsProvider; pub use fee_info::{retain_accounts, FeeError}; -pub use instance_state::{mock, NodeState}; +pub use instance_state::NodeState; pub use state::ProposalValidationError; pub use state::{get_l1_deposits, BuilderValidationError, StateValidationError, ValidatedState}; + +#[cfg(any(test, feature = "testing"))] +pub use instance_state::mock; diff --git a/types/src/v0/mod.rs b/types/src/v0/mod.rs index b0da4bab57..fda9ab1126 100644 --- a/types/src/v0/mod.rs +++ b/types/src/v0/mod.rs @@ -17,12 +17,15 @@ pub mod traits; mod utils; pub use header::Header; pub use impls::{ - get_l1_deposits, mock, retain_accounts, BuilderValidationError, FeeError, - ProposalValidationError, StateValidationError, + get_l1_deposits, retain_accounts, BuilderValidationError, FeeError, ProposalValidationError, + StateValidationError, }; pub use utils::*; use vbs::version::{StaticVersion, StaticVersionType}; +#[cfg(any(test, feature = "testing"))] +pub use impls::mock; + // This is the single source of truth for minor versions supported by this major version. // // It is written as a higher-level macro which takes a macro invocation as an argument and appends