alloy-rs / alloy

Transports, Middleware, and Networks for the Alloy project
https://alloy.rs
Apache License 2.0
634 stars 230 forks source link

[Feature] Make some gas fields u64 for `Header` #1241

Closed tcoratger closed 1 month ago

tcoratger commented 1 month ago

Component

consensus, eips, genesis

Describe the feature you would like

At the moment, some header gas fields are u128:

https://github.com/alloy-rs/alloy/blob/002aed5726e47696edf7e125d11c2794ceaca5e4/crates/consensus/src/header.rs#L65-L105

This includes:

All these fields are considered as u64 in reth which leads to a lot of unnecessary conversions as pointed out here https://github.com/paradigmxyz/reth/pull/10691#issuecomment-2329273121. If there is no particular contraindication, it would be good to make all these fields become u64 directly in alloy to standardize and avoid conversions in reth.

Additional context

No response

klkvr commented 1 month ago

I think if we actually want to do this, any gas fields in alloy should be turned into u64 as otherwise we'd just end up with same conversions but in other places

this will be both breaking and a bit confusing, so would like @mattsse @yash-atreya to confirm that we're fine with doing this in next breaking release before someone starts working on this

also I think base_fee_per_gas should stay as u128, and should be set to u128 in reth too

yash-atreya commented 1 month ago

We had done a numerical audit https://github.com/alloy-rs/alloy/pull/454 https://github.com/alloy-rs/alloy/pull/474 and decided that gas fields should be u128, but makes sense to unify them to u64 since reth and revm both use u64.

also I think base_fee_per_gas should stay as u128, and should be set to u128 in reth too

+1