Add frame_support::crypto::ecdsa::Public.to_eth_address() (k256-based) and use it in pallets (paritytech#11087)

* `ecdsa::Public::to_eth_address` + test, beefy-mmr `convert()` to use it, contracts Ext interface

* `seal_ecdsa_to_eth_address` all but benchmark done

* `seal_ecdsa_to_eth_address` + wasm test

* `seal_ecdsa_to_eth_address` + benchmark

* fixed dependencies

* Apply suggestions from code review

Co-authored-by: Alexander Theißen <[email protected]>

* fixes from review #1

* ecdsa::Public(*pk).to_eth_address() moved to frame_support and contracts to use it

* beefy-mmr to use newly added frame_support function for convertion

* a doc fix

* import fix

* benchmark fix-1 (still fails)

* benchmark fixed

* Apply suggestions from code review

Co-authored-by: Alexander Theißen <[email protected]>

* fixes on Alex T feedback

* to_eth_address() put into extension trait for sp-core::ecdsa::Public

* Update frame/support/src/crypto/ecdsa.rs

Co-authored-by: Alexander Theißen <[email protected]>

* Update frame/contracts/src/wasm/mod.rs

Co-authored-by: Alexander Theißen <[email protected]>

* fixes on issues pointed out in review

* benchmark errors fixed

* fmt fix

* EcdsaRecoverFailed err docs updated

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <[email protected]>

* make applied suggestions compile

* get rid of unwrap() in runtime

* Remove expect

Co-authored-by: Alexander Theißen <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>

Author: 3 people

Cargo.lock

@@ -10,7 +10,6 @@ repository = "https://github.com/paritytech/substrate"
 [dependencies]
 hex = { version = "0.4", optional = true }
 codec = { version = "3.0.0", package = "parity-scale-codec", default-features = false, features = ["derive"] }
-k256 = { version = "0.10.2", default-features = false, features = ["arithmetic"] }
 log = { version = "0.4.13", default-features = false }
 scale-info = { version = "2.0.1", default-features = false, features = ["derive"] }
 serde = { version = "1.0.136", optional = true }
@@ -42,7 +41,6 @@ std = [
     "frame-support/std",
     "frame-system/std",
     "hex",
-    "k256/std",
     "log/std",
     "pallet-beefy/std",
     "pallet-mmr/std",

frame/beefy-mmr/Cargo.toml

@@ -39,7 +39,7 @@ use sp_std::prelude::*;
 use beefy_primitives::mmr::{BeefyDataProvider, BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion};
 use pallet_mmr::{LeafDataProvider, ParentNumberAndHash};
 
-use frame_support::traits::Get;
+use frame_support::{crypto::ecdsa::ECDSAExt, traits::Get};
 
 pub use pallet::*;
 
@@ -71,19 +71,16 @@ where
 pub struct BeefyEcdsaToEthereum;
 impl Convert<beefy_primitives::crypto::AuthorityId, Vec<u8>> for BeefyEcdsaToEthereum {
 	fn convert(a: beefy_primitives::crypto::AuthorityId) -> Vec<u8> {
-		use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey};
-		use sp_core::crypto::ByteArray;
-
-		PublicKey::from_sec1_bytes(a.as_slice())
-			.map(|pub_key| {
-				// uncompress the key
-				let uncompressed = pub_key.to_encoded_point(false);
-				// convert to ETH address
-				sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..].to_vec()
-			})
+		sp_core::ecdsa::Public::try_from(a.as_ref())
 			.map_err(|_| {
 				log::error!(target: "runtime::beefy", "Invalid BEEFY PublicKey format!");
 			})
+			.unwrap_or(sp_core::ecdsa::Public::from_raw([0u8; 33]))
+			.to_eth_address()
+			.map(|v| v.to_vec())
+			.map_err(|_| {
+				log::error!(target: "runtime::beefy", "Failed to convert BEEFY PublicKey to ETH address!");
+			})
 			.unwrap_or_default()
 	}
 }

frame/beefy-mmr/src/lib.rs

@@ -1963,6 +1963,44 @@ benchmarks! {
 		let origin = RawOrigin::Signed(instance.caller.clone());
 	}: call(origin, instance.addr, 0u32.into(), Weight::MAX, None, vec![])
 
+	// Only calling the function itself for the list of
+	// generated different ECDSA keys.
+	seal_ecdsa_to_eth_address {
+		let r in 0 .. API_BENCHMARK_BATCHES;
+		let key_type = sp_core::crypto::KeyTypeId(*b"code");
+		let pub_keys_bytes = (0..r * API_BENCHMARK_BATCH_SIZE)
+			.map(|_| {
+				sp_io::crypto::ecdsa_generate(key_type, None).0
+			})
+		.flatten()
+		.collect::<Vec<_>>();
+		let pub_keys_bytes_len = pub_keys_bytes.len() as i32;
+		let code = WasmModule::<T>::from(ModuleDefinition {
+			memory: Some(ImportedMemory::max::<T>()),
+			imported_functions: vec![ImportedFunction {
+				module: "__unstable__",
+				name: "seal_ecdsa_to_eth_address",
+				params: vec![ValueType::I32, ValueType::I32],
+				return_type: Some(ValueType::I32),
+			}],
+			data_segments: vec![
+				DataSegment {
+					offset: 0,
+					value: pub_keys_bytes,
+				},
+			],
+			call_body: Some(body::repeated_dyn(r * API_BENCHMARK_BATCH_SIZE, vec![
+				Counter(0, 33), // pub_key_ptr
+				Regular(Instruction::I32Const(pub_keys_bytes_len)), // out_ptr
+				Regular(Instruction::Call(0)),
+				Regular(Instruction::Drop),
+			])),
+			.. Default::default()
+		});
+		let instance = Contract::<T>::new(code, vec![])?;
+		let origin = RawOrigin::Signed(instance.caller.clone());
+	}: call(origin, instance.addr, 0u32.into(), Weight::MAX, None, vec![])
+
 	seal_set_code_hash {
 		let r in 0 .. API_BENCHMARK_BATCHES;
 		let code_hashes = (0..r * API_BENCHMARK_BATCH_SIZE)

frame/contracts/src/benchmarking/mod.rs

@@ -22,6 +22,7 @@ use crate::{
 	Pallet as Contracts, Schedule,
 };
 use frame_support::{
+	crypto::ecdsa::ECDSAExt,
 	dispatch::{DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable},
 	storage::{with_transaction, TransactionOutcome},
 	traits::{Contains, Currency, ExistenceRequirement, OriginTrait, Randomness, Time},
@@ -30,7 +31,7 @@ use frame_support::{
 use frame_system::RawOrigin;
 use pallet_contracts_primitives::ExecReturnValue;
 use smallvec::{Array, SmallVec};
-use sp_core::crypto::UncheckedFrom;
+use sp_core::{crypto::UncheckedFrom, ecdsa::Public as ECDSAPublic};
 use sp_io::crypto::secp256k1_ecdsa_recover_compressed;
 use sp_runtime::traits::Convert;
 use sp_std::{marker::PhantomData, mem, prelude::*};
@@ -232,6 +233,9 @@ pub trait Ext: sealing::Sealed {
 	/// Recovers ECDSA compressed public key based on signature and message hash.
 	fn ecdsa_recover(&self, signature: &[u8; 65], message_hash: &[u8; 32]) -> Result<[u8; 33], ()>;
 
+	/// Returns Ethereum address from the ECDSA compressed public key.
+	fn ecdsa_to_eth_address(&self, pk: &[u8; 33]) -> Result<[u8; 20], ()>;
+
 	/// Tests sometimes need to modify and inspect the contract info directly.
 	#[cfg(test)]
 	fn contract_info(&mut self) -> &mut ContractInfo<Self::T>;
@@ -1204,6 +1208,10 @@ where
 		secp256k1_ecdsa_recover_compressed(&signature, &message_hash).map_err(|_| ())
 	}
 
+	fn ecdsa_to_eth_address(&self, pk: &[u8; 33]) -> Result<[u8; 20], ()> {
+		ECDSAPublic(*pk).to_eth_address()
+	}
+
 	#[cfg(test)]
 	fn contract_info(&mut self) -> &mut ContractInfo<Self::T> {
 		self.top_frame_mut().contract_info()
@@ -1267,6 +1275,7 @@ mod tests {
 	use codec::{Decode, Encode};
 	use frame_support::{assert_err, assert_ok};
 	use frame_system::{EventRecord, Phase};
+	use hex_literal::hex;
 	use pallet_contracts_primitives::ReturnFlags;
 	use pretty_assertions::assert_eq;
 	use sp_core::Bytes;
@@ -2718,4 +2727,36 @@ mod tests {
 			));
 		});
 	}
+	#[test]
+	fn ecdsa_to_eth_address_returns_proper_value() {
+		let bob_ch = MockLoader::insert(Call, |ctx, _| {
+			let pubkey_compressed: [u8; 33] =
+				hex!("028db55b05db86c0b1786ca49f095d76344c9e6056b2f02701a7e7f3c20aabfd91")[..]
+					.try_into()
+					.unwrap();
+			assert_eq!(
+				ctx.ext.ecdsa_to_eth_address(&pubkey_compressed).unwrap(),
+				hex!("09231da7b19A016f9e576d23B16277062F4d46A8")[..]
+			);
+			exec_success()
+		});
+
+		ExtBuilder::default().build().execute_with(|| {
+			let schedule = <Test as Config>::Schedule::get();
+			place_contract(&BOB, bob_ch);
+
+			let mut storage_meter = storage::meter::Meter::new(&ALICE, Some(0), 0).unwrap();
+			let result = MockStack::run_call(
+				ALICE,
+				BOB,
+				&mut GasMeter::<Test>::new(GAS_LIMIT),
+				&mut storage_meter,
+				&schedule,
+				0,
+				vec![],
+				None,
+			);
+			assert_matches!(result, Ok(_));
+		});
+	}
 }

frame/contracts/src/exec.rs

@@ -418,6 +418,9 @@ pub struct HostFnWeights<T: Config> {
 	/// Weight of calling `seal_ecdsa_recover`.
 	pub ecdsa_recover: Weight,
 
+	/// Weight of calling `seal_ecdsa_to_eth_address`.
+	pub ecdsa_to_eth_address: Weight,
+
 	/// The type parameter is used in the default implementation.
 	#[codec(skip)]
 	pub _phantom: PhantomData<T>,
@@ -653,6 +656,7 @@ impl<T: Config> Default for HostFnWeights<T> {
 			hash_blake2_128: cost_batched!(seal_hash_blake2_128),
 			hash_blake2_128_per_byte: cost_byte_batched!(seal_hash_blake2_128_per_kb),
 			ecdsa_recover: cost_batched!(seal_ecdsa_recover),
+			ecdsa_to_eth_address: cost_batched!(seal_ecdsa_to_eth_address),
 			_phantom: PhantomData,
 		}
 	}

frame/contracts/src/schedule.rs

@@ -503,6 +503,9 @@ mod tests {
 		fn contract_info(&mut self) -> &mut crate::ContractInfo<Self::T> {
 			unimplemented!()
 		}
+		fn ecdsa_to_eth_address(&self, _pk: &[u8; 33]) -> Result<[u8; 20], ()> {
+			Ok([2u8; 20])
+		}
 	}
 
 	fn execute<E: BorrowMut<MockExt>>(wat: &str, input_data: Vec<u8>, mut ext: E) -> ExecResult {
@@ -1085,6 +1088,42 @@ mod tests {
 		assert_eq!(mock_ext.ecdsa_recover.into_inner(), [([1; 65], [1; 32])]);
 	}
 
+	#[test]
+	#[cfg(feature = "unstable-interface")]
+	fn contract_ecdsa_to_eth_address() {
+		/// calls `seal_ecdsa_to_eth_address` for the contstant and ensures the result equals the
+		/// expected one.
+		const CODE_ECDSA_TO_ETH_ADDRESS: &str = r#"
+(module
+	(import "__unstable__" "seal_ecdsa_to_eth_address" (func $seal_ecdsa_to_eth_address (param i32 i32) (result i32)))
+	(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32)))
+	(import "env" "memory" (memory 1 1))
+
+	(func (export "call")
+		;; fill the buffer with the eth address.
+		(call $seal_ecdsa_to_eth_address (i32.const 0) (i32.const 0))
+
+		;; Return the contents of the buffer
+		(call $seal_return
+			(i32.const 0)
+			(i32.const 0)
+			(i32.const 20)
+		)
+
+		;; seal_return doesn't return, so this is effectively unreachable.
+		(unreachable)
+	)
+	(func (export "deploy"))
+)
+"#;
+
+		let output = execute(CODE_ECDSA_TO_ETH_ADDRESS, vec![], MockExt::default()).unwrap();
+		assert_eq!(
+			output,
+			ExecReturnValue { flags: ReturnFlags::empty(), data: Bytes([0x02; 20].to_vec()) }
+		);
+	}
+
 	const CODE_GET_STORAGE: &str = r#"
 (module
 	(import "seal0" "seal_get_storage" (func $seal_get_storage (param i32 i32 i32) (result i32)))