Repository navigation
Consider changing Weight to be a 1-element tuple struct rather than a type alias to u64 #9584
Description
Nowadays weights are defined as a type alias to u64:
pub type Weight = u64;This could bring in problems for functions that accept a Weight, e.g. the GasMeter constructor in the contracts pallet:
fn new(gas_limit: Weight) -> SelfThere could be a potential hazard where the caller sent in a u64 that does not represent the gas limit, but instead represents some other quantity (such as a block number or an account ID). The compiler currently does not catch such kinds of errors, and will happily compile the code successfully.
More importantly, converting the type alias to a struct is part of the vision of trying to create Chromatic Weights, where not only do we record the weight of the time used for computation, we'd also include the storage used and the heap memory used to execute a particular extrinsic or function.
We could imagine that Chromatic Weights will have the following format:
struct ChromaticWeight {
pub storage: u64,
pub time: u64,
pub memory: u64,
}If I'm not mistaken, the current Weight records only the computation time used, so its value would belong to the time field. PoV size limits can then also be generalized into storage usage and so be recorded into the storage field.
Code: frame/support/src/weights.rs, and many other files that contain a function accepting Weight as a parameter
Activity
ericjmiller commentedon Sep 23, 2021
Is this still important? I was doing some initial investigation into this and the changes would be extremely wide scoping.
There are roughly 4,000 references to the
Weightstruct in substrate, most of them type coercion. Inframe/contracts/src/weights.rsalone, there are ~700 lines with type coercion:Would it be completely crazy to customize the
asoperator forWeightstructs? Otherwise, I see no other good way than to brute force it 4,000 times, which is scary with potential for error.KiChjang commentedon Sep 23, 2021
@ericjmiller Unfortunately you can't customize how the
astype-casting operator works in Rust. I don't think this is as scary as it sounds, because onceWeightbecomes a 1-tuple struct, the compiler would just error instead of silently accepting the code that you have cited, and the job would just be to explicitly convert all integer types into theWeighttype (most likely using.into()), until no more compiler errors are thrown.However, since the change is so extensive, we haven't really had a consensus yet as to whether this is a desirable change. There could be a potential way forward via constructing a nice API around the new
Weightstruct, so that we don't always have to call.into(), but that does require a bit of research to see what the best ergonomics are without sacrificing type safety.gui1117 commentedon Sep 23, 2021
If we strongly type Weight and PoVSize then we can have a single tuple of all metadata of a dispatchable to define all of them:
pallet::weight((Weight, DispatchClass, PaysFee, PoV))could be doable.ascjones commentedon Sep 24, 2021
From a
scale-infometadata perspective, having strong types is much better. Type aliases are erased so it is more difficult to differentiate between au64andWeight. Although the alias will show up in the type name field, there is no guarantee it is the same type.KiChjang commentedon Oct 7, 2021
I've updated the description to this issue so that we now actually have a clear goal of why we'd need this aside from type safety -- we would eventually want to introduce the concept of Chromatic Weights.
kianenigma commentedon Aug 21, 2022
Will be closed by #10918
KiChjang commentedon Sep 8, 2022
This is already done with the advent of #12138.