Merged
Conversation
squadgazzz
approved these changes
Mar 6, 2026
Contributor
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow up to #4238.
While that PR correctly identified the issue that
httpswas 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_httpsunit test to the driver main and running it from there.cargo test -- test_httpspassed whilecargo test -p driver -- test_httpsdid not. This indicates thathttpssupport was only enabled when the whole workspace gets compiled as one. When we only compiled thedriverfeature 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
alloylikes 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 introducealloy_rpc_types_ethandalloy_rpc_types_traceon 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_httpsand it passed. Indicating that theethrpccrate now correctly causes https to be supported in any crate that depends on it.