alloy-rs / core

High-performance, well-tested & documented core libraries for Ethereum, in Rust
https://alloy.rs
Apache License 2.0
764 stars 139 forks source link

Make `Bytes` map to `Bytes` in `SolType` #545

Closed rachel-bousfield closed 6 months ago

rachel-bousfield commented 6 months ago

Motivation

alloy-sol-types exposes a sol_data::T variant for each of the primitive types. For example, alloy-primitives::FixedBytes is represented by an alloy-sol-types::FixedBytes, whose's SolType impl points back to the former via the associated RustType.

impl<const N: usize> SolType for FixedBytes<N>
where
    ByteCount<N>: SupportedFixedBytes,
{
    type RustType = RustFixedBytes<N>;   // alias for the primitive
    // rest of impl   
}

In each case RustType points back to the alloy primitive, except for Bytes. Currently, Bytes instead maps to Vec<u8>.

impl SolType for Bytes {
    type RustType = Vec<u8>;
}

This creates issues for encoding & decoding patterns in SDKs like Stylus, which declare traits like.

pub trait AbiType {
    type SolType: SolType<RustType = Self>;
}

In particular, we lose the bidirectional relationship the other primitives have. The current work-around in Stylus is to declare a second, incompatible Bytes type that implements SolType with a correspondingSolValueType, but this is suboptimal since it's both confusing and not officially supported by Alloy.

/// **Note:** this trait is an implementation detail. As such, it should not
/// be implemented directly unless implementing a custom
/// [`SolType`](crate::SolType), which is also discouraged. Consider
/// using [`SolValue`](crate::SolValue) instead.
ρ trait SolTypeValue<T: super―SolType> { ... }

Solution

The Solution is to make Bytes map to Bytes, which it turns out is a very small diff.

PR Checklist

prestwich commented 6 months ago

the original goal of using Vec<u8> (like using [u8;N] in the past) was to make it more natural for rust developers. i.e. these are the types they would use anyway, with no conversion or new API necessary. Given that we've moved away from that goal over time, I'm fine making this change

Couple things:

rachel-bousfield commented 6 months ago

@prestwich Regarding the first point, there's currently a macro_rules bytes! macro that lets you create a Bytes from string literals. If we were to move to a proc macro, bytes![...] could expand to Bytes::from(vec![...]) for non-str inputs.

macro_rules! bytes {
    () => {
        $crate::Bytes::new()
    };

    ($($s:literal)+) => {{
        // force const eval
        const STATIC_BYTES: &'static [u8] = &$crate::hex!($($s)+);
        $crate::Bytes::from_static(STATIC_BYTES)
    }};
}

For the second point, something that's nice about this is that bytes now expands to Bytes. You'd need to put []uint8 in an event, say, for the generated struct to actually have a Vec<u8> if I understand things correctly.

prestwich commented 6 months ago

you can add a macro rules arm like this:

    [$($inner:literal),+] => {{
        // force const eval
        const STATIC_BYTES: &'static [u8] = &[$($inner),+];
        $crate::Bytes::from_static(STATIC_BYTES)
    }}

    #[test]
    fn bytes_macros() {
        const B1: Bytes = bytes!("010203040506070809");
        const B2: Bytes = bytes![1, 2, 3, 4, 5, 6, 7, 8, 9];

        assert_eq!(B1, B2);
    }