Add missing 256 bits types which are needed by Solang

[seanyoung]

Signed-off-by: Sean Young [email protected]

[seanyoung]

See also polkadot-js/api#2752

[Robbepop]

LGTM

@ascjones ?

[ascjones]

Torn on this since they are not rust primitives. What about using a custom type e.g. U256([u8; 32]), as used in https://github.com/paritytech/parity-common/blob/master/primitive-types/src/lib.rs#L38?

Imagine if I were using this type-info to generate some rust types for decoding, I would have to generate my own U256 type.

I guess would require a bit of extra code to handle that in polkadot-js though.

[seanyoung]

I guess would require a bit of extra code to handle that in polkadot-js though.

I am not following this. polkadot-js can handle u256 and i256 primitives fine, since polkadot-js/api#2752

[seanyoung]

Sorry, I think what you're saying is, these shouldn't be primitives at all. There is something to be said for that, to be fair. What would be helpful is a type which is signed or unsigned of N bytes long, I think.

[Robbepop]

Sorry, I think what you're saying is, these shouldn't be primitives at all. There is something to be said for that, to be fair. What would be helpful is a type which is signed or unsigned of N bytes long, I think.

Are other types such as U512, U420 or I1100 be used at all anywhere? If not we shouldn't over generalize our current framework in my opinion. I can understand both of your positions to be honest. They both make sense when viewing them from different perspectives.

I personally have no favorite solution but we should be very careful what support for fundamental types we add in scale-info. From my knowledge the U256 types are only really used in Ethereum contexts and nowhere else really - please correct me if I am wrong which is probably why Andrew argues that we shouldn't include them as fundamental types in the first place. On the other handside we already have types like u128 that are rarely seen outside of Rust contexts and also as @seanyoung has correctly said it already has built-in support in Polkadot JS and would make things simpler for the UIs as well.

[seanyoung]

I haven't seen U256 or I256 outside of Ethereum, so Solidity, Vyper and Fe.

Having these types in scale-info is very useful as it really simplifies the ABI encoding/decoding: it's simply 32 bytes rather than a compact type. Added to that, the UI already supports this. I'm hoping that this useful enough to tip the balance 😀

clang has support for ints of N bits long -- the LLVM IR has had this feature for a long time. This is exactly what Solang uses
for these types. See http://blog.llvm.org/2020/04/the-new-clang-extint-feature-provides.html It is not inconceivable that one day this feature will come to rust.

I am somewhat arguing from the point of view of what's good for solang/solidity. Looking at the larger picture, I find it harder to favour one solution over another.

[ascjones]

I'm also still torn. Merging on the basis that it looks like the polkadotjs PRs are already in, and if it works then 👍

I think this whole topic needs revisiting because it is a departure from the rest of the Primitives.

lang has support for ints of N bits long -- the LLVM IR has had this feature for a long time

Perhaps that is the solution, since SCALE is language agnostic so should scale-info.

[Robbepop]

I'm also still torn. Merging on the basis that it looks like the polkadotjs PRs are already in, and if it works then +1

I think this whole topic needs revisiting because it is a departure from the rest of the Primitives.

lang has support for ints of N bits long -- the LLVM IR has had this feature for a long time

Perhaps that is the solution, since SCALE is language agnostic so should scale-info.

I would be strictly against this since the scale-info crate isa means to provide meta information about SCALE encodeable types but SCALE does not have any means to encode or decode any N-bit wide (unsigned) integer so scale-info should not be more general than SCALE.

[Robbepop]

I'm also still torn. Merging on the basis that it looks like the polkadotjs PRs are already in, and if it works then +1
I think this whole topic needs revisiting because it is a departure from the rest of the Primitives.

lang has support for ints of N bits long -- the LLVM IR has had this feature for a long time

Perhaps that is the solution, since SCALE is language agnostic so should scale-info.

I would be strictly against this since the scale-info crate isa means to provide meta information about SCALE encodeable types but SCALE does not have any means to encode or decode any N-bit wide (unsigned) integer so scale-info should not be more general than SCALE.

With this regard we should also add SCALE impls for U256 and I256 if not yet existent to the parity-scale-codec crate.

[ascjones]

Yeah this is part of the reason I was torn on this. I'm not certain such types belong in there, perhaps in an external crate or just use https://github.com/paritytech/parity-common/blob/master/primitive-types/src/lib.rs#L38 and add #[derive(Encode. Decode). But that is towards the argument of them not being considered primitives.