desub icon indicating copy to clipboard operation
desub copied to clipboard

Should SubstrateType more consistent with RustTypeMarker ?

Open yjhmelody opened this issue 5 years ago • 1 comments

I noticed that SubstrateType is a stateful version of RustTypeMarker. And I think them should keep similiar as possible.

I think the following common types should be defined in a new enum type.

pub enum SubstrateType {
	/// 512-bit hash type
	H512(primitives::H512),
	/// 256-bit hash type
	H256(primitives::H256),

	/// Recursive Call Type
	Call(Vec<(String, SubstrateType)>),
	/// Era
	Era(runtime_primitives::generic::Era),

	/// Vote
	#[serde(with = "RemoteVote")]
	GenericVote(pallet_democracy::Vote),
 ...

And SubstrateType should be:

pub enum SubstrateType<T> {
     Common(T),
 ...

Other people can compose their own implementations more easily. And can ensure that the core changes as little as possible

How about your thinking?

yjhmelody avatar Dec 14 '20 09:12 yjhmelody

Yup, SubstrateType is a stateful/Output version of RustTypeMarker (RustTypeMarker + bytestring in, SubstrateType out) Sounds like a good addition that would make the types more clear.

In addition to the types you listed, the Data, Address and SignedExtra would also belong on this Common enum since they don't have a direct correspondent in RustTypeMarker

insipx avatar Dec 14 '20 10:12 insipx