EIP2 check

[Dinonard]

Description

Add a missing check EIP2 related check to the pallet-frontier's signature recovery.

[RomarQ]

Adding some more context:

Suggested change

	const SECP256K1N_HALF: H256 = H256([
	/// The order of the secp256k1 curve, divided by two.
	///
	/// Signatures that should be checked according to EIP-2 should have an S value less than or equal to this:
	///
	/// `57896044618658097711785492504343953926418782139537452191302581570759080747168`
	const SECP256K1N_HALF: H256 = H256([

[RomarQ]

Can you add a regression test that should fail if this check is not applied?

Example: https://github.com/leonqadirie/reth/blob/2fb63b04911affa850ada5a336953942f4f11b0f/crates/primitives/src/transaction/signature.rs#L161

[sorpaas]

This isn't necessary. The check happens in TransactionSignature in the ethereum crate. https://github.com/rust-ethereum/ethereum/blob/master/src/transaction/legacy.rs#L103

We already make sure the signature is valid when decoding the value.

But like @RomarQ said, it would be really great to add more docs/tests on this.

[sorpaas]

I think the issue is the EIP-7702 implementation? It doesn't seem to be using TransactionSignature type but just raw values.

[sorpaas]

Ah, and EIP-1559 and EIP-2930. Only the legacy transaction type is implemented correctly!

[sorpaas]

I'll take over the fixes (it needs to happen on ethereum crate but not here). I'll also issue a security advisory because it's a specification deviation.

• Low severity for Frontier, because signature malleability itself is not actually a security issue. The advise is simply to upgrade the runtime following normal schedule.
• High severity for Ethereum ecosystem for those who use the ethereum crate, because it's a specification deviation.

[sorpaas]

See rust-ethereum/ethereum#67

[Dinonard]

Thank you.
I'll close the PR since it'll be handled in the ethereum crate.