search

alloy-rs / core

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

Implement specific bit types for integers #677

Closed rory-ocl closed 3 months ago

rory-ocl commented 5 months ago

Use generic integer types with specific bit sizes to allow 1:1 mapping between rust and solidity types.

Motivation

I am doing some work on OffchainLabs/stylus-sdk-rs and am having some issues with the way that integer types are currently implemented here. For stylus, we require a 1:1 type mapping between rust types and solidity types. This breaks down for us because in alloy, currently, there is many solidity types that map to each rust integer type. For instance, both uint24 and uint32 in solidity map to u32 in rust.

Solution

Our proposed solution is to use alloy_primitves::{Signed, Uint} with the proper number of bits for each integer type.

PR Checklist

Note

This is currently WIP as there are a few tests that are currently panicking due to failed library assertions. I believe the issue is in the int_impls! macro in core/crates/sol-types/src/types/data_type.rs. I need to review the usage of the generic bit sizes that are used here. I also need to check the try_from implementation used in a couple other tests.

joshuacolvin0 commented 5 months ago

The motivation for this PR is the same as #545

DaniPopes commented 4 months ago

OK with this, however this will have worse devex as ruint has worse APIs for conversions with primitives. Will look into the failures as well this week and get a breaking release out.

joshuacolvin0 commented 4 months ago

OK with this, however this will have worse devex as ruint has worse APIs for conversions with primitives. Will look into the failures as well this week and get a breaking release out.

Note that we have already worked around the issue for now with this PR that will be merged soon: https://github.com/OffchainLabs/stylus-sdk-rs/pull/137

The concern about worse devex is certainly valid, and definitely open to other possible suggestions.

DaniPopes commented 4 months ago

It's also intuitive that u24 -> U24 rather than u32, but there is also the concern that storing it is 8 bytes instead of 3/4

I'm OK with it TBH

wdyt @prestwich @gakonst @mattsse

also FYI if you update to latest alloy versions we have made the string representation of types const too, so you can use it in your const ABI stuff: https://docs.rs/alloy-sol-types/latest/alloy_sol_types/trait.SolType.html#associatedconstant.SOL_NAME

mattsse commented 4 months ago

reasonable imo