Remove 256bit primitives and deprecated calls

[jsdw]

Remove TypeDefPrimitive::I256 and TypeDefPrimitive::U256 variants (which on its own required no other changes).

Since this will be a major breaking change, I removed the deprecated calls too.

@ascjones whaddya reckon?

[ascjones]

LGTM. Although let's hear from @xermicus before we do this, to ensure that they can live without this for solang.

[jsdw]

Makes sense; based on that old PR to add them I get that this could be contentious.

But yeah, a collection of reasons in favour of removing them offhand:

• They aren't rust primitives and so don't make sense as scale-info primitives,
• Because they aren't rust primitives, they don't play well with things like serde (which has no notion of 256bit types, leading to these being handled in some weird way) or whatever.
• We use the primitive-types::U256 etc types which have TypeInfo derives and are treated as newtype wrappers around fixed length arrays rather than using these specifal 256bit primitives
• And what about higher-number-of-bits primitives anyway. Better to not make an exception in the TypeInfo we present just for 256 bit ones and to take the same approach for all fixed-length-byte things.
• Would simplify handling in places like scale-encode, scale-decode etc which also inherit this odd "map this TypeInfo primitive type into a non-primitive rust type unlike all other mappings". ie the inconsistency is infectious.
• Things like Subxt or whatever that may want to provide ergonomic interfaces for 256bit types can already do so by either having U256/I256 types that go From/Into the generated U256([u8;32]) and I256([u8;32]) types anyway (ie we can use the typeinfo paths etc to spot and work with arbitrary custom-length integer types).

[xermicus]

Fine by me if it's treated as a breaking change for a major release.

[seanyoung]

This would break the Solang build, the U256 and I256 is used here:

https://github.com/hyperledger/solang/blob/main/src/abi/substrate.rs#L66
and
https://github.com/hyperledger/solang/blob/main/src/abi/substrate.rs#L75

How else are we going to describe 256 bit integer types in the metadata for Solidity?

[xermicus]

How else are we going to describe 256 bit integer types in the metadata for Solidity?

That's a very good point; My assumption was that there will be a different way of representing them (in the next major release, together with this change), and if the coherent solution is to remove it as a primitive type first, I'm fine with it. Ultimately we asked for supporting it, not to just to get rid of it 😁

@jsdw @ascjones what do you have in mind and when will the next major release be out? A way to describe an integer type with varying bit length would be nice. Solidity knows signed and unsigned integer types of arbitrary size stepped by 8bits up to 256bits and currently we can't reflect that.

We'll need some time as well to adapt other front-ends (polkadot-js) and the ink-metadata schema (which I think also requires a breaking change / major release).

[jsdw]

How else are we going to describe 256 bit integer types in the metadata for Solidity?

Solidity knows signed and unsigned integer types of arbitrary size stepped by 8bits up to 256bits and currently we can't reflect that.

To me it feels like these points go hand in hand; I don't feel like we should add primitive types for every 8 bit increment to scale-info (these also wouldn't be valid rust primitives), so I think that supporting 256 bit types (also not a rust primitive) and supporting 8-bit increments falls into the same camp and should be handled in the same way.

Currently, we expose a bunch of int-like types in https://github.com/paritytech/parity-common/blob/master/primitive-types/src/lib.rs, and the TypeInfo for types encode is essentially a newtype wrapper around an array of bytes.

If you wanted to support arbitrary int/uint sizes, one approach today would be to do something similar and, given a type like:

struct UInt<const N: usize>([u8; N]);

One would implement TypeInfo on it to describe it as something like:

scale_info::Type {
    path: vec!["foo", "UInt"],
    type_def: TypeDefComposite {
        fields: vec![
            Field {
                ty: scale_info::Type {
                    type_def: TypeDefArray {
                        len: N,
                        type_params: scale_info::Type {
                            ty: TypeDefPrimitive::U8
                        }
                    }     
                }
            }
        ]
    }
}

Without this scale-info::TypeInfo, the type could already be SCALE encoded or decoded just fine. With it, code could match on the unique path to this type and inspect length of the byte array to know precisely how many bits the number is supposed to be and such.

I'm not super familiar with your requirements, and so if that isn't suitable then I'd be interested to hear why not and understand the issue better :)

In general, the downside of this approach is that there is no "built-in" support for such types in scale-info, ie you need some custom logic to look out for and handle the relevant types based on their paths. The upside is that this approach is super versatile and allows for all sorts of custom handling of types which otherwise don't fit neatly into the scale-codec/scale-info type model, and the type model remains a fairly simple mapping to rust types :)

[xermicus]

@jsdw Thanks, I think that makes all sense and is very similar to what I had in mind. Straight off the bat I don't see why turning it basically into an array of bytes should not work.

We need to stick to some path and serialization method. Ideally, serialization stays the same as is, making this change only breaking in regards to the metadata. I guess the path is what all front-end libraries need to identify this as a number and not a byte array (not sure how else this can be done). This is the only downside I see for now, by (ab)using the u256 primitive, it was implicitly clear that it's a number.

[seanyoung]

@jsdw that does look like a good solution. Would make sense to hold off on merging this PR until we have the Solang side sorted and we're sure it's going to work? We need to make sure it works fine in e.g. polkadot-js

[jsdw]

From my side I'm in no rush to merge this PR, so please do take the time you need to explore the approach and comment back with your verdict :)

[jsdw]

@seanyoung do you have any updates from your side; I'm keen to know how you've been getting on!

[xermicus]

It's on our backlog but we currently lack resources for this. An external contributor might look into it, I can check the status.