Skip to content

Dont rely on feature unification#4240

Merged
MartinquaXD merged 2 commits intomainfrom
dont-rely-on-feature-unification
Mar 6, 2026
Merged

Dont rely on feature unification#4240
MartinquaXD merged 2 commits intomainfrom
dont-rely-on-feature-unification

Conversation

@MartinquaXD
Copy link
Contributor

@MartinquaXD MartinquaXD commented Mar 6, 2026

Description

Follow up to #4238.
While that PR correctly identified the issue that https was not enabled when is should have been it did not resolve it correctly. It incorrectly added the tls feature on the workspace toml assuming this will cause the feature to always be enabled but that's actually not correct.
The driver still did not support https. I was able to reproduce that by copying the new test_https unit test to the driver main and running it from there.
cargo test -- test_https passed while cargo test -p driver -- test_https did not. This indicates that https support was only enabled when the whole workspace gets compiled as one. When we only compiled the driver feature unification was not there to pull in the necessary features.

This PR addresses this by explicitly enabling all the necessary features such that every crate can be compiled and tested individually without complaints.

Changes

Add a bunch of features that were previously assumed to not be needed because feature unification pulled them in without our knowledge.

Since alloy likes to follow the pattern of using feature flags to enable compilation of sub-crates I avoided the use of feature flags by depending on the underlying sub-crate whenever possible. For example this caused me to introduce alloy_rpc_types_eth and alloy_rpc_types_trace on the workspace level.

This PR only fixes the symptom but doesn't prevent us from relying on feature unification without our knowledge again. This should be addressed in a follow up PR.

How to test

After that was done I repeated the test with cargo -p driver -- test_https and it passed. Indicating that the ethrpc crate now correctly causes https to be supported in any crate that depends on it.

@MartinquaXD MartinquaXD requested a review from a team as a code owner March 6, 2026 12:23
@MartinquaXD MartinquaXD enabled auto-merge March 6, 2026 12:25
Copy link
Contributor

@m-sz m-sz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses an issue with Cargo's feature unification by making crate dependencies and their features explicit in each Cargo.toml. This ensures each crate can be built and tested independently. The changes involve adding specific features to alloy and other dependencies across multiple crates, and updating use statements accordingly. No critical issues were found in this implementation.

@MartinquaXD MartinquaXD added this pull request to the merge queue Mar 6, 2026
Merged via the queue into main with commit 4b1a8f1 Mar 6, 2026
19 checks passed
@MartinquaXD MartinquaXD deleted the dont-rely-on-feature-unification branch March 6, 2026 12:46
@github-actions github-actions bot locked and limited conversation to collaborators Mar 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants