Skip to content

Conversation

michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Sep 9, 2025

Ring contains a call to OPENSSL_cpuid_setup which is not supported by Miri [it can't handle most FFI].

This PR feature-gates ring so that we can build ethereum_hashing without it. This enables us to run the test suites for tree_hash and ssz_types using Miri, which is increasingly important with the introduction of unsafe code in:

This change is unfortunately not backwards-compatible, as downstream users compiling with default-features = false might have their build broken by the exclusion of ring (on a non-x86_64 platform). Therefore this PR necessitates a minor version bump.

Copy link

codecov bot commented Sep 9, 2025

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.00%. Comparing base (64c47ad) to head (f7bde56).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/lib.rs 0.00% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main      #15   +/-   ##
=======================================
  Coverage   50.00%   50.00%           
=======================================
  Files           2        2           
  Lines          70       70           
=======================================
  Hits           35       35           
  Misses         35       35           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@michaelsproul
Copy link
Member Author

Unfortunately it's hard to get better coverage here without providing another feature flag to disable sha2 and force ring usage. Currently ring will only be used on non-x86_64 machines, or where SHA extensions are not available. Our Linux CI runner is x86_64 and has sha extensions.

@michaelsproul michaelsproul merged commit 5d604e9 into main Sep 10, 2025
13 of 14 checks passed
@michaelsproul michaelsproul deleted the no-ring branch September 10, 2025 03:18
@DragonDev1906
Copy link
Contributor

Thank you for this change. I'm in a similar situation where I can't use ring as a dependency and thus had to use a custom API compatible implementation without it (because a dependency used it).
Always thought this wouldn't happen because this crate is specifically for abstracting over the implementation, so I didn't bother creating an issue for a ring feature flag.

@michaelsproul
Copy link
Member Author

Glad to hear!

What are you working on these days @DragonDev1906?

@DragonDev1906
Copy link
Contributor

DragonDev1906 commented Sep 10, 2025

Ethereum block verification in a TEE, which does use ssz + tree_hash. And ring unfortunately doesn't support the x86_64-fortanix-unknown-sgx target, so all crates that depend on it are problematic.

Used to build data bridging for other chains by checking its integrity in the TEE and signing the response, and to detect on-chain events for use in TEE-based L2+bridging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants