bugfix: eip-191 hash calculation fix (#2248)

# Goal The goal of this PR is to make sure the hash calculation for eip-191 is compatible with JS libraries. Closes #2247 # Discussion - Changed eip-191 message hash calculation to match js implementation in popular Ethereum libraries # Checklist - [X] Unit Tests added? - [X] Spec version incremented?
frequency-chain · Jan 3, 2025 · 8945c5e · 8945c5e
1 parent c5151d5
commit 8945c5e
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 40 deletions.
diff --git a/common/primitives/src/signatures.rs b/common/primitives/src/signatures.rs
@@ -5,7 +5,7 @@ use frame_support::{
 	__private::{codec, RuntimeDebug},
 	pallet_prelude::{Decode, Encode, MaxEncodedLen, TypeInfo},
 };
-use scale_info::prelude::format;
+use parity_scale_codec::alloc::string::ToString;
 use sp_core::{
 	crypto,
 	crypto::{AccountId32, FromEntropy},
@@ -19,6 +19,9 @@ use sp_runtime::{
 	MultiSignature,
 };
 
+/// Ethereum message prefix eip-191
+const ETHEREUM_MESSAGE_PREFIX: &[u8; 26] = b"\x19Ethereum Signed Message:\n";
+
 /// Signature verify that can work with any known signature types.
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[derive(Eq, PartialEq, Clone, Encode, Decode, MaxEncodedLen, RuntimeDebug, TypeInfo)]
@@ -254,10 +257,11 @@ fn check_secp256k1_signature(signature: &[u8; 65], msg: &[u8; 32], signer: &Acco
 	}
 }
 
-fn eth_message_hash(message: &str) -> [u8; 32] {
-	let prefixed = format!("{}{}{}", "\x19Ethereum Signed Message:\n", message.len(), message);
-	log::debug!(target:"ETHEREUM", "prefixed {:?}",prefixed);
-	sp_io::hashing::keccak_256(prefixed.as_bytes())
+fn eth_message_hash(message: &[u8]) -> [u8; 32] {
+	let only_len = (message.len() as u32).to_string().into_bytes();
+	let concatenated = [ETHEREUM_MESSAGE_PREFIX.as_slice(), only_len.as_slice(), message].concat();
+	log::debug!(target:"ETHEREUM", "prefixed {:?}",concatenated);
+	sp_io::hashing::keccak_256(concatenated.as_slice())
 }
 
 fn check_ethereum_signature<L: Lazy<[u8]>>(
@@ -270,21 +274,14 @@ fn check_ethereum_signature<L: Lazy<[u8]>>(
 	};
 
 	// signature of ethereum prefixed message eip-191
-	let message_prefixed = eth_message_hash(&format!("0x{:?}", HexDisplay::from(&msg.get())));
+	let message_prefixed = eth_message_hash(&msg.get());
 	if verify_signature(&signature.as_ref(), &message_prefixed, signer) {
 		return true
 	}
 
 	// signature of raw payload, compatible with polkadotJs signatures
 	let hashed = sp_io::hashing::keccak_256(&msg.get());
-	if verify_signature(signature.as_ref(), &hashed, signer) {
-		return true
-	}
-
-	// frequency wrapped for Metamask compatibility
-	let frequency_wrapped =
-		eth_message_hash(&format!("<Frequency>0x{:?}</Frequency>", HexDisplay::from(&msg.get())));
-	verify_signature(&signature.as_ref(), &frequency_wrapped, signer)
+	verify_signature(signature.as_ref(), &hashed, signer)
 }
 
 #[cfg(test)]
@@ -308,14 +305,14 @@ mod tests {
 	#[test]
 	fn ethereum_prefixed_eip191_signatures_should_work() {
 		// payload is random and the signature is generated over that payload by a standard EIP-191 signer
-		let payload = from_hex("0x0a0300e659a7a1628cdd93febc04a4e0646ea20e9f5f0ce097d9a05290d4a9e054df4e028c7d0a3500000000830000000100000026c1147602cf6557f4e0068a78cd4b22b6f6b03e106d05618cde8537e4ffe454c1f285c69f563934857a63463571d57723fbad6ac7de44611ed674f02c04c2ae00").expect("Should convert");
-		let signature_raw = from_hex("0x056ca64d31251a1f20733ce2a741e2963c87a9674a35f8619b6b97210ae8c8b54c2853da1b943dd95ac3b893b37f69ca7dc38c13f8ec92a235b00ae03426505900").expect("Should convert");
+		let payload = b"test eip-191 message payload";
+		let signature_raw = from_hex("0x276dcc9c69da24dd8441ba3acc9b60d8aae0cb39f0bc5ad92c723a31bf11575031d860978280191a0a97a1f74336ca0c79a8b1b3aab013fb58a27f113b73b2081b").expect("Should convert");
 		let unified_signature = UnifiedSignature::from(ecdsa::Signature::from_raw(
 			signature_raw.try_into().expect("should convert"),
 		));
 
 		let public_key = ecdsa::Public::from_raw(
-			from_hex("0x025b107c7f38d5ac7d618e626f9fa57eec683adf373b1352cd20e5e5c684747079")
+			from_hex("0x03be5b145e12c5fb95151374ed919eb445ade57637d729dd2d73bf161d4bc10329")
 				.expect("should convert")
 				.try_into()
 				.expect("invalid size"),
@@ -343,25 +340,6 @@ mod tests {
 		assert!(unified_signature.verify(&payload[..], &unified_signer.into_account()));
 	}
 
-	#[test]
-	fn ethereum_custom_wrapped_signatures_should_work() {
-		// payload is random and the signature is generated over that payload by Metamask signer
-		let payload = from_hex("0x0a0300e659a7a1628cdd93febc04a4e0646ea20e9f5f0ce097d9a05290d4a9e054df4e028c7d0a3500000000830000000100000026c1147602cf6557f4e0068a78cd4b22b6f6b03e106d05618cde8537e4ffe4548de1bcb12a1d42e58b218a7abb03cb629111625cf3449640d837c5aa98b87d8e00").expect("Should convert");
-		let signature_raw = from_hex("0x9633e747bcd951bdb9d98ff84c65562e1f62bd059c578a942859e1695f2472aa0dbaab48c28f6dbc795baa73c27252d97e8dc2170fd7d69694d5cd1863fb968c00").expect("Should convert");
-		let unified_signature = UnifiedSignature::from(ecdsa::Signature::from_raw(
-			signature_raw.try_into().expect("should convert"),
-		));
-
-		let public_key = ecdsa::Public::from_raw(
-			from_hex("0x025b107c7f38d5ac7d618e626f9fa57eec683adf373b1352cd20e5e5c684747079")
-				.expect("should convert")
-				.try_into()
-				.expect("invalid size"),
-		);
-		let unified_signer = UnifiedSigner::from(public_key);
-		assert!(unified_signature.verify(&payload[..], &unified_signer.into_account()));
-	}
-
 	#[test]
 	fn ethereum_invalid_signatures_should_fail() {
 		let payload = from_hex("0x0a0300e659a7a1628cdd93febc04a4e0646ea20e9f5f0ce097d9a05290d4a9e054df4e028c7d0a3500000000830000000100000026c1147602cf6557f4e0068a78cd4b22b6f6b03e106d05618cde8537e4ffe4548de1bcb12a1d42e58b218a7abb03cb629111625cf3449640d837c5aa98b87d8e00").expect("Should convert");

diff --git a/e2e/package-lock.json b/e2e/package-lock.json
diff --git a/pallets/passkey/src/tests_v2.rs b/pallets/passkey/src/tests_v2.rs
@@ -11,7 +11,7 @@ use crate::test_common::{
 	utilities::*,
 };
 use pallet_balances::Call as BalancesCall;
-use sp_core::{sr25519, sr25519::Public, Pair};
+use sp_core::{bytes::from_hex, ecdsa, sr25519, sr25519::Public, Pair};
 use sp_runtime::{traits::One, DispatchError::BadOrigin};
 
 struct TestPasskeyPayloadBuilder {
@@ -523,3 +523,38 @@ fn pre_dispatch_with_exceeding_weight_should_fail() {
 		assert_err!(v, InvalidTransaction::ExhaustsResources);
 	});
 }
+
+#[test]
+fn passkey_example_should_work() {
+	new_test_ext().execute_with(|| {
+		// arrange
+		let account_id = AccountId32::new(from_hex("0x000000000000000000000000cf613044ccd8c1c60f561b99bd1fd2daef89625f").unwrap().try_into().unwrap());
+		let pass_key_public_key = PasskeyPublicKey(from_hex("0x029bd263885e5eeaea31fa3b2e78ab1106d2cb1995045777fca3b38913a755d250").unwrap().try_into().unwrap());
+		let payload = PasskeyPayloadV2 {
+			passkey_public_key: pass_key_public_key,
+			verifiable_passkey_signature: VerifiablePasskeySignature {
+				signature: from_hex("0x30440220498c9d752c98f4cec77bb386f48dc52ccc6523f386c8942b3d50ff9dbdb4046a02205d2f2e3fa8bf27fd5c8f0d7f1ae8724a30dab1dac758019459114a447357c54b").unwrap().try_into().unwrap(),
+				authenticator_data: from_hex("0x49960de5880e8c687434170f6476605b8fe4aeb9a28632c7995cf3ba831d97630500000003").unwrap().try_into().unwrap(),
+				client_data_json: from_hex("0x7b2274797065223a22776562617574686e2e676574222c226368616c6c656e6765223a227372576f2d4a49393564336332393362774e756469747a53687a4f636849577a584d336478376d554f4351222c226f726967696e223a22687474703a2f2f6c6f63616c686f73743a3536353135222c2263726f73734f726967696e223a66616c73657d").unwrap().try_into().unwrap(),
+			},
+			account_ownership_proof: MultiSignature::Ecdsa(ecdsa::Signature::from_raw(from_hex("0xa0d9b3fe775b4be9d2b52f37f8a21c30be037a858f85cca82bce58ac05a95d9621063978dc086a83532e839882e701f2b7134541a4814526c888b8650c3eb5f81b").unwrap().try_into().unwrap())),
+			passkey_call: PasskeyCallV2 {
+				account_id: account_id.clone(),
+				account_nonce: 0,
+				call: Box::new(RuntimeCall::System(SystemCall::remark { remark: vec![1, 2, 3u8] })),
+			},
+		};
+		assert_ok!(Balances::force_set_balance(
+			RawOrigin::Root.into(),
+			account_id.into(),
+			10000000000
+		));
+
+		// act
+		let res =
+			Passkey::validate_unsigned(TransactionSource::InBlock, &Call::proxy_v2 { payload });
+
+		// assert
+		assert!(res.is_ok());
+	});
+}
diff --git a/runtime/frequency/src/lib.rs b/runtime/frequency/src/lib.rs
@@ -401,7 +401,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("frequency"),
 	impl_name: create_runtime_str!("frequency"),
 	authoring_version: 1,
-	spec_version: 140,
+	spec_version: 141,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,
@@ -415,7 +415,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("frequency-testnet"),
 	impl_name: create_runtime_str!("frequency"),
 	authoring_version: 1,
-	spec_version: 140,
+	spec_version: 141,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,