bug: arc-consensus-types does not build with --no-default-features; `signer-local` gates a mandatory dependency

[mehmetkr-31]

Summary

arc-consensus-types does not build with --no-default-features, and the signer-local feature that causes it provides no actual optionality.

Reproduction

On main:

$ cargo check -p arc-consensus-types --no-default-features
error[E0432]: unresolved import `malachitebft_signing_ed25519`
  --> crates/types/src/proposal_part.rs:26:5
error[E0432]: unresolved import `malachitebft_signing_ed25519`
  --> crates/types/src/codec/proto.rs:28:5
error[E0432]: unresolved import `malachitebft_signing_ed25519`
  --> crates/types/src/signing.rs:21:9
error[E0432]: unresolved import `malachitebft_signing_ed25519`
  --> crates/types/src/ssz/v1/vote.rs:21:5
error: could not compile `arc-consensus-types` (lib) due to 4 previous errors

Cause

[features]
default = ["signer-local"]
signer-local = ["dep:malachitebft-signing-ed25519"]

The four modules above import malachitebft_signing_ed25519 unconditionally, and feature = "signer-local" never appears in a #[cfg] anywhere in the workspace — it exists only to gate the dependency. So the feature cannot meaningfully be turned off: with it the crate builds, without it the build breaks with unresolved imports rather than a diagnosable message.

Worth contrasting with arc-signer, which has a genuine choice between signing providers and states it properly:

#[cfg(not(any(feature = "local", feature = "remote")))]
compile_error!("At least one signing provider feature must be enabled");

cargo check -p arc-signer --no-default-features fails with that sentence instead of an import error. arc-consensus-types has no such choice to express.

Suggested fix

Make malachitebft-signing-ed25519 a required dependency and drop signer-local along with the now-empty default. No crate in the workspace requests signer-local explicitly, so nothing needs updating alongside it.

Opened as #237. One thing worth your call: removing the feature name means an external consumer who spelled out features = ["signer-local"] would get an unknown-feature error, even though no such consumer can exist today (the crate does not build without it). If you would rather preserve the name, keeping signer-local = [] as a no-op alias achieves the same fix while staying spelling-compatible — happy to switch to that. The other alternative is to keep the feature and add a compile_error! guard like arc-signer's, which turns the failure into a clear message but leaves a flag that can only ever be on.

Note

This is the second feature-declaration gap in this crate; see also #233 (the arbitrary feature does not build standalone). Both are invisible to CI, which only ever builds the full workspace with --all-features.

[osr21]

Verified the claims independently before commenting:

• Reproduction confirmed at the declaration level: signer-local = ["dep:malachitebft-signing-ed25519"] gates the dep, while signing.rs, proposal_part.rs, codec/proto.rs, and ssz/v1/vote.rs all import malachitebft_signing_ed25519 with no #[cfg] — including a pub use re-export in signing.rs, meaning the "optional" dependency's types are unconditionally part of this crate's public API (Ed25519, PrivateKey, PublicKey, Signature). That's worth adding to the record because it settles the design question: a feature can't meaningfully gate a dependency whose types the crate re-exports unconditionally. The choice isn't between "optional dep" and "required dep" — the dep has been API-load-bearing all along, and the feature flag was only ever bookkeeping.
• The "no workspace consumer" claim survives a scrutiny the issue text invites: a repo code search for signer-local returns hits in spammer/Cargo.toml, quake/Cargo.toml, and the root manifest — but all three are alloy-signer-local (an unrelated Alloy crate) substring-matching. No workspace crate enables the feature by name, so fix(types): make malachitebft-signing-ed25519 a required dependency #237's removal breaks nothing internal. Anyone re-verifying this later: search for "signer-local" with the quotes-and-bracket context or you'll get the same false positives.

On the open design call in the issue — removal is right, and the no-op alias is worth skipping. The compatibility argument for signer-local = [] protects a consumer class that provably cannot exist: any external crate that wrote features = ["signer-local"] also compiled the four unconditional imports, which only ever succeeded with the dep present — i.e., the feature enabled. Nobody can be depending on the off-state, and the on-state's spelling disappears harmlessly at the next semver-meaningful event anyway (the crate inherits publish but isn't on crates.io yet, per #233's discussion). Keeping a dead alias trades permanent reader confusion ("what does this gate?") for compatibility with zero users. The compile_error! option is strictly worse here — arc-signer earns its guard because it has a real either/or between providers; this crate has no choice to express, and a guard would formalize a flag that can only be on.

The pattern worth extracting across #233 + this: both bugs are the same species — feature declarations whose truth is only tested under workspace-wide --all-features unification. #233 was "feature on doesn't build standalone"; this is "feature off never builds at all." cargo hack check --each-feature --no-dev-deps -p <crate> catches both directions (it tests the empty-feature case too, which plain --each-feature alone covers via its default-off runs). If the cargo-hack CI job proposed on #233 gets filed, it should include --no-default-features coverage explicitly — that's the half of the class this issue represents, and it's the half the #233 discussion didn't cover.

#237's shape (make the dep required, drop the feature and the now-empty default) matches the evidence; will review the diff there.