fix(types): make the arbitrary feature self-contained

[mehmetkr-31]

Fixes #233.

Summary

The arbitrary feature of arc-consensus-types does not build on its own:

$ cargo check -p arc-consensus-types --features arbitrary
error[E0433]: failed to resolve: could not find `Arbitrary` in `arbitrary`
  --> crates/types/src/address.rs:39:53
   |
39 | #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
   |                                                     ^^^^^^^^^ could not find `Arbitrary` in `arbitrary`

The feature gates derive(arbitrary::Arbitrary) on Address, but declares neither of the two things that derive needs:

1. arbitrary/derive — the derive macro itself lives behind that feature, hence the E0433 above.
2. alloy-primitives/arbitrary — without it the generated impl fails with the trait bound alloy_primitives::Address: Arbitrary<'_> is not satisfied, since the wrapped type has no Arbitrary impl of its own.

Why CI does not catch this

arc-consensus-db and arc-node-consensus both depend on alloy-rpc-types-engine with features = ["arbitrary"]. In a workspace build, feature unification turns on exactly what this crate omitted, so --all-features at the workspace level compiles and the gap stays hidden. It only surfaces when the crate is built alone — which is also how a consumer would build it, and crates/types inherits publish from the workspace rather than opting out.

Change

Declare both, following the pattern arc-evm and arc-node already use, where the arbitrary feature forwards to the alloy dependencies explicitly:

arbitrary = ["dep:arbitrary", "alloy-primitives/arbitrary"]
arbitrary = { workspace = true, optional = true, features = ["derive"] }

Testing

• cargo check -p arc-consensus-types --features arbitrary — now compiles (this is the command that fails on main).
• cargo check --workspace --all-features — still passes, no regression.
• cargo test -p arc-consensus-types --features arbitrary — 192 tests pass.
• cargo fmt --all --check — clean.
• Cargo.lock is unchanged: arbitrary's derive feature and alloy-primitives/arbitrary are both already enabled somewhere in the graph, so no version resolution moves.

I left the workspace-level arbitrary = "1.3" in the root Cargo.toml alone deliberately — adding derive there would enable it for every consumer of the workspace dependency, and only this crate needs it.

[osr21]

Disclosure: I'm not affiliated with Circle — an external community contributor, not a maintainer. I have no write access to this repository, so any review state I set (approval or change request) carries no merge authority and is advisory only. Please treat this as one contributor's technical assessment, and defer to Circle maintainers for the binding review.

---

Verified everything independently against main before reviewing:

• The broken state is as described: arbitrary = ["dep:arbitrary"] with no derive and no alloy-primitives/arbitrary forwarding, while address.rs gates derive(arbitrary::Arbitrary) on the feature. Both missing pieces are real and both are needed — fixing only one moves the error from E0433 to the E0277 bound failure, exactly as the issue documents.
• The arc-evm precedent holds: its arbitrary feature forwards to the alloy/revm/reth deps explicitly, so this change follows existing workspace convention rather than inventing one.
• The masking analysis is the most valuable part of the PR and it's correct, with one detail worth making explicit: this only stays hidden because workspace-level commands (--workspace --all-features) unify features across all members. With resolver v2, cargo check -p <crate> resolves only that crate's own feature graph — which is why the standalone check is both the honest reproduction and a valid verification that the fix is complete (no unification bailing it out).

Two notes, neither blocking:

1. Placement of derive is equivalent to forwarding, with one subtle property worth knowing. Putting features = ["derive"] on the optional dependency declaration (rather than "arbitrary/derive" in the feature array) works because features on an optional dep only activate when the dep does. The one behavioral difference: if a second feature ever enables dep:arbitrary for a different purpose, it inherits derive whether it wants it or not, whereas the feature-array form keeps the choice per-feature. For a proc-macro feature on a fuzzing dep, that's a non-concern in practice — just noting it's a deliberate trade, and the current form is the more readable of the two.

2. This bug class is systematic, and there's a cheap guard. Feature-unification masking will re-occur for any crate whose optional features are exercised in CI only via workspace-wide builds — this crate just happens to be the first one caught building standalone. cargo hack check --each-feature -p <crate> (or workspace-wide cargo hack check --each-feature --workspace, which is the standard tool for exactly this) would catch the whole class in CI. Worth a follow-up issue rather than this PR, but without it the next #[cfg_attr(feature = ...)] derive added to any crate can silently regress the same way.

The restraint on the workspace-level arbitrary = "1.3" is also correct — pushing derive into the workspace declaration would force the proc-macro dependency on every consumer to fix one crate's declaration gap.

Minimal, correct, well-evidenced. Approving.

[mehmetkr-31]

Rebased onto main after the v0.8.0 sync — head is 00c4cce. The conflict was the same one #237 hit: byzantine = ["dep:malachitebft-engine-byzantine"] was added to the [features] block. Both lines kept, nothing else touched.

Re-measured on the new base rather than carrying the old numbers over:

# main (v0.8.0)
$ cargo check -p arc-consensus-types --features arbitrary --locked
error[E0433]: cannot find `Arbitrary` in `arbitrary`

# this branch
$ cargo check -p arc-consensus-types --features arbitrary --locked
Finished `dev` profile in 8.61s

So the gap survived the sync: v0.8.0 edited this file only to add byzantine, leaving arbitrary = ["dep:arbitrary"] and the derive-less dependency untouched. Cargo.lock still unchanged, cargo fmt --all --check clean.

This stays independent of #237 — that one removes default and signer-local, this one only widens the arbitrary feature, and after both rebases they no longer touch the same lines. Either order works now.

[osr21]

Re-verified the rebase (00c4cce) against current main — the approval carries over, with one correction on the #237 independence claim.

Confirmed:

• The diff is content-identical to what I approved: 2+/2− on crates/types/Cargo.toml only, same two edits (alloy-primitives/arbitrary forwarding in the feature, features = ["derive"] on the optional dep). Cargo.lock is not in the diff, matching the claim.
• The premise survived the sync, confirmed from chore: sync v0.8.0 to arc-node #285's actual patch for this file: in the [features] block the sync added only byzantine, leaving arbitrary = ["dep:arbitrary"] and the derive-less dependency line byte-identical. (For precision: the sync also added alloy-eips and the optional malachitebft-engine-byzantine lines in [dependencies] — neither near your edits, so the point stands.)
• Merges cleanly onto current main in isolation, reproducing the same 2+/2− diff.

The correction — "either order works now" is not what git reports. I merge-tested against #237's fresh rebase (e68f4eb, pushed a few minutes before this comment) on current main, in both orders. Both conflict:

<<<<<<< HEAD
arbitrary = ["dep:arbitrary"]
=======
default = ["signer-local"]
arbitrary = ["dep:arbitrary", "alloy-primitives/arbitrary"]
>>>>>>> pr231

"No longer touch the same lines" is literally true — but git merges hunks, not lines, and #237 deletes default = ["signer-local"], the line directly above the one this PR edits. Adjacent-line changes overlap into one hunk, so whichever PR lands second needs a rebase. The same holds for #301 (which duplicates #237's change), tested both orders as well.

To be clear about the size of this: the resolution is trivial and obvious — keep both edits — and the two changes are still semantically independent, so this is a sequencing note for whoever merges, not a flaw in either PR. But a maintainer landing one should expect the other to go red on mergeability rather than composing silently.