Consider changing Weight to be a 1-element tuple struct rather than a type alias to u64

[KiChjang]

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) -> Self

There 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

[ericjmiller]

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 Weight struct in substrate, most of them type coercion. In frame/contracts/src/weights.rs alone, there are ~700 lines with type coercion:

  fn on_initialize_per_trie_key(k: u32, ) -> Weight {
          (0 as Weight)
                  // Standard Error: 2_000
                  .saturating_add((2_178_000 as Weight).saturating_mul(k as Weight))
                  .saturating_add(T::DbWeight::get().reads(1 as Weight))
                  .saturating_add(T::DbWeight::get().writes(1 as Weight))
                  .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(k as Weight)))

Would it be completely crazy to customize the as operator for Weight structs? Otherwise, I see no other good way than to brute force it 4,000 times, which is scary with potential for error.

[KiChjang]

@ericjmiller Unfortunately you can't customize how the as type-casting operator works in Rust. I don't think this is as scary as it sounds, because once Weight becomes 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 the Weight type (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 Weight struct, 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]

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]

From a scale-info metadata perspective, having strong types is much better. Type aliases are erased so it is more difficult to differentiate between a u64 and Weight. Although the alias will show up in the type name field, there is no guarantee it is the same type.

[KiChjang]

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]

Will be closed by #10918

[KiChjang]

This is already done with the advent of #12138.