remove serde derive for BlockId

[yjhmelody]

The BlockId serde is never used in substrate.
And I think it's a bad design. It could not be used in rpc api.

[yjhmelody]

BTW, What's the best practice to support a rpc both for BlockNumber and Hash?

[bkchr]

BTW, What's the best practice to support a rpc both for BlockNumber and Hash?

substrate/primitives/rpc/src/number.rs

Lines 33 to 40 in 4be5dd2

	#[derive(Copy, Clone, Serialize, Deserialize, Debug, PartialEq)]
	#[serde(untagged)]
	pub enum NumberOrHex {
	/// The number represented directly.
	Number(u64),
	/// Hex representation of the number.
	Hex(U256),
	}

[bkchr]

The BlockId serde is never used in substrate.

You don't know what downstream is doing. Having there serde is not harming anyone. Please revert, but we can keep the const fn change.

[yjhmelody]

BTW, What's the best practice to support a rpc both for BlockNumber and Hash?

substrate/primitives/rpc/src/number.rs

Lines 33 to 40 in 4be5dd2

	#[derive(Copy, Clone, Serialize, Deserialize, Debug, PartialEq)]
	#[serde(untagged)]
	pub enum NumberOrHex {
	/// The number represented directly.
	Number(u64),
	/// Hex representation of the number.
	Hex(U256),
	}

Very strange, the hex is not about hash but block number( and if it's about block hash, it must be u256, could not be other. e.g. u512. Although u256 is main stream).

[bkchr]

Very strange, the hex is not about hash but block number

Yeah you are right, I mixed this up. All RPCs also work with hash, besides the one that can give you a block number for a hash. In general, using hash is much better to address blocks to ensure that you are accessing the correct block.

[yjhmelody]

It's very easy for downstream to define a old BlockId type and impl From if they need.

The BlockId even have not #[serde(untagged)], I do not think anyone will use the serde derive

[yjhmelody]

I think it's better to also derive deserialize and add #[serde(untagged)].
But this leads to a silent behavior change, so I think remove the serde derive first, then after a while, add it back as needed

[bkchr]

And I think it's a bad design. It could not be used in rpc api.

If you give some better description than just "bad design", I'm okay with merging this.

[yjhmelody]

i don't know what you want to express

There is no such word in the title.

I can close this PR. I just think that BlockId can be integrated into many substrate and third parties RPCs in the future

[bkchr]

You are saying it is a bad design and I just want to understand why it is a bad design. Not more.

[yjhmelody]

You are saying it is a bad design and I just want to understand why it is a bad design. Not more.

Hi, I means it have no #[serde(untagged)], I think no one will use it when serialize it to json. And it doesn't even support deserialization.

If someone did, I think it is also very bad, it should be kept in the same style as NumberOrHex