feat: native keccak feature flag

[rachel-bousfield]

Background

As outlined in Paradigm's introduction of Alloy, avoiding fragmentation of the Rust ecosystem on Ethereum means that everyone should use a consistent set of primitive types. As Rust expands into new Ethereum domains like smart contract development, generality will be key for this shared dependency.

The alloy_primatives crate included here does this very well, but could be made even better for upcoming Rust-enabled Ethereum smart contract environments like Arbitrum Stylus.

In addition to providing types, the crate currently pulls in tiny_keccak. This makes sense for most existing platforms, but in code running on top of VMs it's reasonable to accelerate common operations through so-called "host I/Os" or "traps". For example, the Stylus model includes a trap for keccak256.

#[link(wasm_import_module = "vm_hooks")]
extern "C" {
    fn native_keccak256(bytes: *const u8, len: usize, output: *mut u8);
}

Because host I/O operations run the function natively, we can get the native performance of tiny_keccak in a VM setting without any of the contribution to binary size. Though the example here is from Stylus, this reasoning applies to other VMs that might want to add hooks for operations like keccak. Anyone can expose a hook in their VM fulfilling native_keccak256 above and accelerate hashing.

Solution

This small PR introduces a new feature flag, native-keccak, that when enabled substitutes tiny_keccak for an extern import. Since the feature is not enabled by default, this change is 100% backward compatible.

Though the changes are to private functions, I've tried to include exhaustive documentation. Please let me know if there's anything I've missed in opening this PR and I'll be happy to make changes.

[DaniPopes]

This is really nice, thank you!
Currently CI is failing because it builds with --all-features which would enable this feature. We can get around this by introducing a second feature for tiny-keccak that is enabled by default, and that takes precedence over native-keccak256 with the cfg_if! like so:

pub fn keccak256<T: AsRef<[u8]>>(bytes: T) -> FixedBytes<32> {
    cfg_if::cfg_if! {
        if #[cfg(feature = "tiny-keccak")] {
            fn keccak256 ...
        } else if #[cfg(feature = "native-keccak")] {
            extern "C" {
                ...
            }
            fn keccak256 ...
        } else {
            compile_error!("At least one Keccak-256 feature must be enabled")
        }
    }

    keccak256(bytes.as_ref())
}

This would also allow us to also expand this in the future with more Keccak implementations. Would you be willing to implement this?
Also, don't worry about breaking changes as we're still pre 1.0 and we'll be releasing 0.3 very soon.

[prestwich]

@DaniPopes is this something that could/should be addressed with a "global keccak backend" abstraction? seems like overkill for now 🤔

[rachel-bousfield]

I'm happy to go any route y'all think. Other options include

• changing the feature flags used in CI
• having instead a tiny_keccak feature enabled by default that opts out of using a native keccak (this would be the only feature flag, since native-keccak would be redundant)

[DaniPopes]

We want to gate the tiny_keccak dep in case of this new feature, so you end up with 2 backends if you enable both features with --all-features, so yes

[rachel-bousfield]

@DaniPopes edited my last comment for clarity: what I meant to say was that there's an in-between option where with no features enabled you use the native keccak hook, but replace it with tiny_keccak should the tiny-keccak feature flag be enabled. So you'd only have 1 feature flag instead of 2.

I have no strong feelings on the mechanism and am happy to implement whichever solution y'all think is best :)

EDIT I'm seeing that there's also a --no-default-features test too. Ok, this method doesn't work then

[rachel-bousfield]

On further reflection we have a few issues

1. CI tests with both --no-default-features and --all-features. Hence, we can't just error if neither flag is enabled unless we're willing to modify CI.
2. If we add a tiny_keccak flag, then those working in a VM setting have an issue. While alloy-primatives by itself will work as expected, if the smart contract author then pulls in another library that depends on alloy with the default features enabled, suddenly everything is now using tiny_keccak instead of the VM hook.

[rachel-bousfield]

Thanks everyone for your help with this. I've included a commit to add a tiny-keccak feature flag.

Importantly, neither tiny-keccak nor native-keccak are enabled by default. However, the effects of tiny-keccak are applied when neither are. This way --no-default-features and --all-features still work as expected.

But because neither are enabled by default, there's no worry that smart contract devs might accidentally stop using native-keccak by pulling in other crates that depend on Alloy.

I think it's a win-win :)

[DaniPopes]

SGTM. I think it's still better we use cfg_if for readability, where now the condition is any(tiny-keccak, not(native-keccak)) with a comment referencing why we are doing this.
Also needs cargo +nightly fmt.

[joshuacolvin0]

LGTM