Skip to content

Commit

Permalink
Use espresso-types without testing feature (#2283)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
sveitser authored Nov 18, 2024
1 parent 187f337 commit c197b21
Show file tree
Hide file tree
Showing 19 changed files with 106 additions and 70 deletions.
42 changes: 42 additions & 0 deletions .github/workflows/cargo-features.yml
Original file line number Diff line number Diff line change
@@ -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
6 changes: 4 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
8 changes: 2 additions & 6 deletions builder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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 }
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
3 changes: 2 additions & 1 deletion flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion hotshot-state-prover/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
3 changes: 3 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions marketplace-builder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ authors = { workspace = true }
edition = { workspace = true }

[features]
default = ["libp2p"]
libp2p = ["sequencer/libp2p"]
testing = ["hotshot-query-service", "sequencer-utils", "tempfile"]

[dependencies]
Expand All @@ -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 }
Expand All @@ -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 }
6 changes: 3 additions & 3 deletions marketplace-solver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ edition = "2021"

[features]
testing = [
"hotshot-query-service",
"hotshot-query-service/testing",
"portpicker",

]

[dependencies]
Expand All @@ -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 }
Expand All @@ -40,4 +39,5 @@ tracing = { workspace = true }
vbs = { workspace = true }

[dev-dependencies]
marketplace-solver = { path = ".", features = [ "testing" ] }
portpicker = { workspace = true }
3 changes: 3 additions & 0 deletions node-metrics/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
20 changes: 15 additions & 5 deletions sequencer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand All @@ -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 }
Expand Down Expand Up @@ -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 }

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

Expand Down
17 changes: 8 additions & 9 deletions sequencer/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -536,28 +536,28 @@ pub mod test_helpers {
network_config: Option<TestConfig<{ NUM_NODES }>>,
}

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,
}
}
}

impl<const NUM_NODES: usize>
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,
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1344,7 +1343,7 @@ mod test {
};
use super::*;
use crate::{
catchup::StatePeers,
catchup::{NullStateCatchup, StatePeers},
persistence::no_storage,
testing::{TestConfig, TestConfigBuilder},
};
Expand All @@ -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();
Expand Down
Loading

0 comments on commit c197b21

Please sign in to comment.