From 4c5f9fe3e2db891390f42215b2078a140e1d59d2 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Thu, 17 Mar 2022 12:39:55 +0200 Subject: [PATCH 01/26] `ecdsa::Public::to_eth_address` + test, beefy-mmr `convert()` to use it, contracts Ext interface --- Cargo.lock | 14 ++++++++++++++ frame/beefy-mmr/src/lib.rs | 16 +++------------- frame/contracts/src/exec.rs | 9 ++++++++- frame/contracts/src/wasm/mod.rs | 3 +++ primitives/core/Cargo.toml | 1 + primitives/core/src/ecdsa.rs | 32 +++++++++++++++++++++++++++++++- 6 files changed, 60 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e2321d7fa99d8..7031837a656d0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1780,6 +1780,7 @@ checksum = "d0d69ae62e0ce582d56380743515fefaf1a8c70cec685d9677636d7e30ae9dc9" dependencies = [ "der", "elliptic-curve", + "rfc6979", "signature", ] @@ -7806,6 +7807,17 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "448296241d034b96c11173591deaa1302f2c17b56092106c1f92c1bc0183a8c9" +[[package]] +name = "rfc6979" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96ef608575f6392792f9ecf7890c00086591d29a83910939d430753f7c050525" +dependencies = [ + "crypto-bigint", + "hmac 0.11.0", + "zeroize", +] + [[package]] name = "ring" version = "0.16.20" @@ -9534,6 +9546,7 @@ version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "02658e48d89f2bec991f9a78e69cfa4c316f8d6a6c4ec12fae1aeb263d486788" dependencies = [ + "digest 0.9.0", "rand_core 0.6.2", ] @@ -9907,6 +9920,7 @@ dependencies = [ "hex", "hex-literal", "impl-serde", + "k256", "lazy_static", "libsecp256k1", "log 0.4.14", diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index 476589717e06c..c97574c0ab517 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -72,19 +72,9 @@ where pub struct BeefyEcdsaToEthereum; impl Convert> for BeefyEcdsaToEthereum { fn convert(a: beefy_primitives::crypto::AuthorityId) -> Vec { - 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() - }) - .map_err(|_| { - log::error!(target: "runtime::beefy", "Invalid BEEFY PublicKey format!"); - }) + sp_core::ecdsa::Public::from(a) + .to_eth_address() + .map(|v| v.to_vec()) .unwrap_or_default() } } diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 455665687d973..06892ad193e17 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -30,7 +30,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}; use sp_io::crypto::secp256k1_ecdsa_recover_compressed; use sp_runtime::traits::Convert; use sp_std::{marker::PhantomData, mem, prelude::*}; @@ -224,6 +224,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; @@ -1175,6 +1178,10 @@ where secp256k1_ecdsa_recover_compressed(&signature, &message_hash).map_err(|_| ()) } + fn ecdsa_to_eth_address(&self, pk: &[u8; 33]) -> Result<[u8; 20], ()> { + ecdsa::Public(*pk).to_eth_address() + } + #[cfg(test)] fn contract_info(&mut self) -> &mut ContractInfo { self.top_frame_mut().contract_info() diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 3912a936684c2..2b4a46a90cbb9 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -496,6 +496,9 @@ mod tests { fn contract_info(&mut self) -> &mut crate::ContractInfo { unimplemented!() } + fn ecdsa_to_eth_address(&self, _pk: &[u8; 33]) -> Result<[u8; 20], ()> { + Ok([0u8; 20]) + } } fn execute>(wat: &str, input_data: Vec, mut ext: E) -> ExecResult { diff --git a/primitives/core/Cargo.toml b/primitives/core/Cargo.toml index 55d00362033d0..129d6406cb595 100644 --- a/primitives/core/Cargo.toml +++ b/primitives/core/Cargo.toml @@ -45,6 +45,7 @@ futures = { version = "0.3.19", optional = true } dyn-clonable = { version = "0.9.0", optional = true } thiserror = { version = "1.0.30", optional = true } bitflags = "1.3" +k256 = { version = "0.10.2", default-features = false, features = ["ecdsa"] } # full crypto ed25519-dalek = { version = "1.0.1", default-features = false, features = ["u64_backend", "alloc"], optional = true } diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index 7a4e4399913dc..5635163b87335 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -43,6 +43,7 @@ use secp256k1::{ ecdsa::{RecoverableSignature, RecoveryId}, Message, PublicKey, SecretKey, }; + #[cfg(feature = "std")] use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "full_crypto")] @@ -58,7 +59,6 @@ pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecds"); type Seed = [u8; 32]; /// The ECDSA compressed public key. -#[cfg_attr(feature = "full_crypto", derive(Hash))] #[derive( Clone, Copy, @@ -71,6 +71,7 @@ type Seed = [u8; 32]; PartialEq, PartialOrd, Ord, + Hash, )] pub struct Public(pub [u8; 33]); @@ -99,6 +100,24 @@ impl Public { }; pubkey.map(|k| Self(k.serialize())).map_err(|_| ()) } + + /// Converts self into Ethereum address + #[cfg_attr(not(feature = "std"), no_std)] + pub fn to_eth_address(&self) -> Result<[u8; 20], ()> { + use crate::hashing::keccak_256; + use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; + + PublicKey::from_sec1_bytes(self.as_slice()) + .map(|pub_key| { + // uncompress the key + let uncompressed = pub_key.to_encoded_point(false); + // convert to ETH address + let res: [u8; 20] = + keccak_256(&uncompressed.as_bytes()[1..])[12..].try_into().unwrap(); + res + }) + .map_err(|_| ()) + } } impl ByteArray for Public { @@ -863,4 +882,15 @@ mod test { let key = sig.recover_prehashed(&msg).unwrap(); assert_ne!(pair.public(), key); } + + #[test] + fn to_eth_address_works() { + let pair = Pair::from_string( + "0x9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60", + None, + ) + .unwrap(); + let eth_address = pair.public().to_eth_address(); + assert_eq!(eth_address.unwrap(), hex!("09231da7b19A016f9e576d23B16277062F4d46A8")[..]); + } } From 414c40ee356f67867fba37e9674531fcec85fc3e Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Mon, 21 Mar 2022 18:17:55 +0200 Subject: [PATCH 02/26] `seal_ecdsa_to_eth_address` all but benchmark done --- frame/contracts/src/exec.rs | 33 +++++++++++++++++++++++++++++ frame/contracts/src/schedule.rs | 4 ++++ frame/contracts/src/wasm/runtime.rs | 26 +++++++++++++++++++++++ frame/contracts/src/weights.rs | 23 ++++++++++++++++++++ 4 files changed, 86 insertions(+) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 06892ad193e17..e64551e6115a1 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -1245,6 +1245,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; @@ -2640,4 +2641,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 = ::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::::new(GAS_LIMIT), + &mut storage_meter, + &schedule, + 0, + vec![], + None, + ); + assert_matches!(result, Ok(_)); + }); + } } diff --git a/frame/contracts/src/schedule.rs b/frame/contracts/src/schedule.rs index 8535166a6ac5c..ff9408d1e449f 100644 --- a/frame/contracts/src/schedule.rs +++ b/frame/contracts/src/schedule.rs @@ -412,6 +412,9 @@ pub struct HostFnWeights { /// 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, @@ -645,6 +648,7 @@ impl Default for HostFnWeights { 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, } } diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 043b45e6a76ed..90b006b94c565 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -219,6 +219,9 @@ pub enum RuntimeCosts { /// Weight of calling `seal_set_code_hash` #[cfg(feature = "unstable-interface")] SetCodeHash, + /// Weight of calling `ecdsa_to_eth_address` + #[cfg(feature = "unstable-interface")] + EcdsaToEthAddress, } impl RuntimeCosts { @@ -299,6 +302,8 @@ impl RuntimeCosts { CallRuntime(weight) => weight, #[cfg(feature = "unstable-interface")] SetCodeHash => s.set_code_hash, + #[cfg(feature = "unstable-interface")] + EcdsaToEthAddress => s.ecdsa_to_eth_address, }; RuntimeToken { #[cfg(test)] @@ -2004,4 +2009,25 @@ define_env!(Env, , Ok(()) => Ok(ReturnCode::Success) } }, + + // Calculates Ethereum address from the ECDSA compressed public key and stores + // it into the supplied buffer + // + // # Parameters + // + // - key_ptr: a pointer to the ECDSA compressed public key + // + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. + [__unstable__] seal_ecdsa_to_eth_address(ctx, key_ptr: u32, out_ptr: u32, out_len_ptr: u32) => { + ctx.charge_gas(RuntimeCosts::EcdsaToEthAddress)?; + let compressed_key: [u8; 33] = + ctx.read_sandbox_memory_as(key_ptr)?; + + Ok(ctx.write_sandbox_output( + out_ptr, out_len_ptr, &ctx.ext.ecdsa_to_eth_address(&compressed_key).encode(), false, already_charged + )?) + }, ); diff --git a/frame/contracts/src/weights.rs b/frame/contracts/src/weights.rs index b438ad51cbfca..0dd071270e1e1 100644 --- a/frame/contracts/src/weights.rs +++ b/frame/contracts/src/weights.rs @@ -101,6 +101,7 @@ pub trait WeightInfo { fn seal_hash_blake2_128(r: u32, ) -> Weight; fn seal_hash_blake2_128_per_kb(n: u32, ) -> Weight; fn seal_ecdsa_recover(r: u32, ) -> Weight; + fn seal_ecdsa_to_eth_address(r: u32, ) -> Weight; fn seal_set_code_hash(r: u32, ) -> Weight; fn instr_i64const(r: u32, ) -> Weight; fn instr_i64load(r: u32, ) -> Weight; @@ -782,6 +783,17 @@ impl WeightInfo for SubstrateWeight { // Storage: Contracts ContractInfoOf (r:1 w:1) // Storage: Contracts CodeStorage (r:1 w:0) // Storage: Timestamp Now (r:1 w:0) + fn seal_ecdsa_to_eth_address(r: u32, ) -> Weight { + (272_893_000 as Weight) + // Standard Error: 1_438_000 + .saturating_add((15_412_877_000 as Weight).saturating_mul(r as Weight)) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) + } + // Storage: System Account (r:1 w:0) + // Storage: Contracts ContractInfoOf (r:1 w:1) + // Storage: Contracts CodeStorage (r:1 w:0) + // Storage: Timestamp Now (r:1 w:0) // Storage: Contracts OwnerInfoOf (r:36 w:36) fn seal_set_code_hash(r: u32, ) -> Weight { (0 as Weight) @@ -1673,6 +1685,17 @@ impl WeightInfo for () { // Storage: Contracts ContractInfoOf (r:1 w:1) // Storage: Contracts CodeStorage (r:1 w:0) // Storage: Timestamp Now (r:1 w:0) + fn seal_ecdsa_to_eth_address(r: u32, ) -> Weight { + (272_893_000 as Weight) + // Standard Error: 1_438_000 + .saturating_add((15_412_877_000 as Weight).saturating_mul(r as Weight)) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + } + // Storage: System Account (r:1 w:0) + // Storage: Contracts ContractInfoOf (r:1 w:1) + // Storage: Contracts CodeStorage (r:1 w:0) + // Storage: Timestamp Now (r:1 w:0) // Storage: Contracts OwnerInfoOf (r:36 w:36) fn seal_set_code_hash(r: u32, ) -> Weight { (0 as Weight) From a7453f4addf81c1ac643009d69b8bcb225137c67 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Mon, 21 Mar 2022 20:26:02 +0200 Subject: [PATCH 03/26] `seal_ecdsa_to_eth_address` + wasm test --- frame/contracts/src/wasm/mod.rs | 48 +++++++++++++++++++++++++++++ frame/contracts/src/wasm/runtime.rs | 6 ++-- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 2b4a46a90cbb9..f532ed489c219 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -1081,6 +1081,54 @@ 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 i32))) + (import "env" "memory" (memory 1 1)) + + ;; size of our buffer is 20 bytes + (data (i32.const 20) "\20") + + (func $assert (param i32) + (block $ok + (br_if $ok + (get_local 0) + ) + (unreachable) + ) + ) + + (func (export "call") + ;; fill the buffer with the eth address. + (call $seal_ecdsa_to_eth_address (i32.const 0) (i32.const 0) (i32.const 20)) + + ;; assert out_len == 20 + (call $assert + (i32.eq + (i32.load (i32.const 20)) + (i32.const 20) + ) + ) + ;; assert that the mock returned 20 zero-bytes + (call $assert + (i64.eq + (i64.load (i32.const 0)) + (i64.const 0x000000000000000000000000000000000000000) + ) + ) + ) + (func (export "deploy")) +) +"#; + + assert_ok!(execute(CODE_ECDSA_TO_ETH_ADDRESS, vec![], MockExt::default())); + } + const CODE_GET_STORAGE: &str = r#" (module (import "seal0" "seal_get_storage" (func $seal_get_storage (param i32 i32 i32) (result i32))) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 90b006b94c565..aeec998460b32 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -2025,9 +2025,7 @@ define_env!(Env, , ctx.charge_gas(RuntimeCosts::EcdsaToEthAddress)?; let compressed_key: [u8; 33] = ctx.read_sandbox_memory_as(key_ptr)?; - - Ok(ctx.write_sandbox_output( - out_ptr, out_len_ptr, &ctx.ext.ecdsa_to_eth_address(&compressed_key).encode(), false, already_charged - )?) + let result = ctx.ext.ecdsa_to_eth_address(&compressed_key).unwrap(); + Ok(ctx.write_sandbox_output(out_ptr, out_len_ptr, &result, false, already_charged)?) }, ); From 7e040fde8cfbd434c5ec154cceec7ff80ff6f367 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Mon, 21 Mar 2022 21:00:26 +0200 Subject: [PATCH 04/26] `seal_ecdsa_to_eth_address` + benchmark --- frame/contracts/src/benchmarking/mod.rs | 38 +++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 8539978bd6b39..dcf9ed314fd8a 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -1910,6 +1910,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 = (0..r * API_BENCHMARK_BATCH_SIZE) + .map(|_| { + sp_io::crypto::ecdsa_generate(key_type, None).encode() + }) + .collect::>(); + let pub_keys_bytes = pub_keys.iter().map(|a| a.encode()).flatten().collect::>(); + let code = WasmModule::::from(ModuleDefinition { + memory: Some(ImportedMemory::max::()), + imported_functions: vec![ImportedFunction { + module: "__unstable__", + name: "seal_ecdsa_to_eth_address", + params: vec![ValueType::I32, ValueType::I32, ValueType::I32], + return_type: None, + }], + data_segments: vec![ + DataSegment { + offset: 24, + value: pub_keys_bytes, + }, + ], + call_body: Some(body::repeated_dyn(r * API_BENCHMARK_BATCH_SIZE, vec![ + Counter(20, 33), // pub_key_ptr + Regular(Instruction::I32Const(0)), // out_ptr + Regular(Instruction::I32Const(4)), // out_len_ptr + Regular(Instruction::Call(0)), + Regular(Instruction::Drop), + ])), + .. Default::default() + }); + let instance = Contract::::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) From d3090c7e87349308592376a509b5bb4e45f54f9c Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Tue, 22 Mar 2022 12:43:42 +0200 Subject: [PATCH 05/26] fixed dependencies --- primitives/core/Cargo.toml | 3 +-- primitives/core/src/ecdsa.rs | 1 - primitives/core/src/lib.rs | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/primitives/core/Cargo.toml b/primitives/core/Cargo.toml index 129d6406cb595..c213e5cc16813 100644 --- a/primitives/core/Cargo.toml +++ b/primitives/core/Cargo.toml @@ -46,6 +46,7 @@ dyn-clonable = { version = "0.9.0", optional = true } thiserror = { version = "1.0.30", optional = true } bitflags = "1.3" k256 = { version = "0.10.2", default-features = false, features = ["ecdsa"] } +sp-core-hashing = { version = "4.0.0", default-features = false, path = "./hashing" } # full crypto ed25519-dalek = { version = "1.0.1", default-features = false, features = ["u64_backend", "alloc"], optional = true } @@ -59,7 +60,6 @@ libsecp256k1 = { version = "0.7", default-features = false, features = ["static- merlin = { version = "2.0", default-features = false, optional = true } secp256k1 = { version = "0.21.2", default-features = false, features = ["recovery", "alloc"], optional = true } ss58-registry = { version = "1.11.0", default-features = false } -sp-core-hashing = { version = "4.0.0", path = "./hashing", default-features = false, optional = true } sp-runtime-interface = { version = "6.0.0", default-features = false, path = "../runtime-interface" } [dev-dependencies] @@ -134,7 +134,6 @@ full_crypto = [ "hex", "libsecp256k1", "secp256k1", - "sp-core-hashing", "sp-runtime-interface/disable_target_static_assertions", "merlin", ] diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index 5635163b87335..717246f5a2aae 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -102,7 +102,6 @@ impl Public { } /// Converts self into Ethereum address - #[cfg_attr(not(feature = "std"), no_std)] pub fn to_eth_address(&self) -> Result<[u8; 20], ()> { use crate::hashing::keccak_256; use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; diff --git a/primitives/core/src/lib.rs b/primitives/core/src/lib.rs index b7c8b69e8a0ab..36eca35d7fdfb 100644 --- a/primitives/core/src/lib.rs +++ b/primitives/core/src/lib.rs @@ -48,11 +48,11 @@ pub use sp_debug_derive::RuntimeDebug; #[cfg(feature = "std")] pub use impl_serde::serialize as bytes; -#[cfg(feature = "full_crypto")] pub mod hashing; +pub use hashing::keccak_256; #[cfg(feature = "full_crypto")] -pub use hashing::{blake2_128, blake2_256, keccak_256, twox_128, twox_256, twox_64}; +pub use hashing::{blake2_128, blake2_256, twox_128, twox_256, twox_64}; pub mod crypto; pub mod hexdisplay; From 869cf84104ae4439de8eb999b87b723ffb5c31b5 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Tue, 22 Mar 2022 19:15:43 +0200 Subject: [PATCH 06/26] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexander Theißen --- frame/beefy-mmr/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index c97574c0ab517..17a9a04fd4bc8 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -75,6 +75,9 @@ impl Convert> for BeefyEcdsaToEth sp_core::ecdsa::Public::from(a) .to_eth_address() .map(|v| v.to_vec()) + .map_err(|| { + log::error!(target: "runtime::beefy", "Invalid BEEFY PublicKey format!"); + }) .unwrap_or_default() } } From c4b6a29aaa44072ca9ec5c4d8012ee7f823dcb47 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Tue, 22 Mar 2022 19:12:59 +0200 Subject: [PATCH 07/26] fixes from review #1 --- Cargo.lock | 29 ----------------------------- frame/beefy-mmr/Cargo.toml | 2 -- primitives/core/Cargo.toml | 3 ++- primitives/core/src/ecdsa.rs | 7 +++---- primitives/core/src/lib.rs | 4 ++-- 5 files changed, 7 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88513718b0dd1..8b96a7367967c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -462,12 +462,6 @@ version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "904dfeac50f3cdaba28fc6f57fdcddb75f49ed61346676a78c4ffe55877802fd" -[[package]] -name = "base64ct" -version = "1.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "874f8444adcb4952a8bc51305c8be95c8ec8237bb0d2e78d2e039f771f8828a0" - [[package]] name = "beef" version = "0.5.1" @@ -5665,7 +5659,6 @@ dependencies = [ "frame-system", "hex", "hex-literal", - "k256", "log 0.4.14", "pallet-beefy", "pallet-mmr", @@ -7073,17 +7066,6 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" -[[package]] -name = "pkcs8" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7cabda3fb821068a9a4fab19a683eac3af12edf0f34b94a8be53c4972b8149d0" -dependencies = [ - "der", - "spki", - "zeroize", -] - [[package]] name = "pkg-config" version = "0.3.19" @@ -9263,7 +9245,6 @@ checksum = "08da66b8b0965a5555b6bd6639e68ccba85e1e2506f5fbb089e93f8a04e1a2d1" dependencies = [ "der", "generic-array 0.14.4", - "pkcs8", "subtle", "zeroize", ] @@ -10476,16 +10457,6 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" -[[package]] -name = "spki" -version = "0.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "44d01ac02a6ccf3e07db148d2be087da624fea0221a16152ed01f0496a6b0a27" -dependencies = [ - "base64ct", - "der", -] - [[package]] name = "ss58-registry" version = "1.11.0" diff --git a/frame/beefy-mmr/Cargo.toml b/frame/beefy-mmr/Cargo.toml index 19703fa79863a..bf72e295c478f 100644 --- a/frame/beefy-mmr/Cargo.toml +++ b/frame/beefy-mmr/Cargo.toml @@ -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 } @@ -43,7 +42,6 @@ std = [ "frame-support/std", "frame-system/std", "hex", - "k256/std", "log/std", "pallet-beefy/std", "pallet-mmr-primitives/std", diff --git a/primitives/core/Cargo.toml b/primitives/core/Cargo.toml index c213e5cc16813..129d6406cb595 100644 --- a/primitives/core/Cargo.toml +++ b/primitives/core/Cargo.toml @@ -46,7 +46,6 @@ dyn-clonable = { version = "0.9.0", optional = true } thiserror = { version = "1.0.30", optional = true } bitflags = "1.3" k256 = { version = "0.10.2", default-features = false, features = ["ecdsa"] } -sp-core-hashing = { version = "4.0.0", default-features = false, path = "./hashing" } # full crypto ed25519-dalek = { version = "1.0.1", default-features = false, features = ["u64_backend", "alloc"], optional = true } @@ -60,6 +59,7 @@ libsecp256k1 = { version = "0.7", default-features = false, features = ["static- merlin = { version = "2.0", default-features = false, optional = true } secp256k1 = { version = "0.21.2", default-features = false, features = ["recovery", "alloc"], optional = true } ss58-registry = { version = "1.11.0", default-features = false } +sp-core-hashing = { version = "4.0.0", path = "./hashing", default-features = false, optional = true } sp-runtime-interface = { version = "6.0.0", default-features = false, path = "../runtime-interface" } [dev-dependencies] @@ -134,6 +134,7 @@ full_crypto = [ "hex", "libsecp256k1", "secp256k1", + "sp-core-hashing", "sp-runtime-interface/disable_target_static_assertions", "merlin", ] diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index 717246f5a2aae..3832ea21738f3 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -59,6 +59,7 @@ pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecds"); type Seed = [u8; 32]; /// The ECDSA compressed public key. +#[cfg_attr(feature = "full_crypto", derive(Hash))] #[derive( Clone, Copy, @@ -71,7 +72,6 @@ type Seed = [u8; 32]; PartialEq, PartialOrd, Ord, - Hash, )] pub struct Public(pub [u8; 33]); @@ -103,8 +103,7 @@ impl Public { /// Converts self into Ethereum address pub fn to_eth_address(&self) -> Result<[u8; 20], ()> { - use crate::hashing::keccak_256; - use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; + use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; PublicKey::from_sec1_bytes(self.as_slice()) .map(|pub_key| { @@ -112,7 +111,7 @@ impl Public { let uncompressed = pub_key.to_encoded_point(false); // convert to ETH address let res: [u8; 20] = - keccak_256(&uncompressed.as_bytes()[1..])[12..].try_into().unwrap(); + sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..].try_into().unwrap(); res }) .map_err(|_| ()) diff --git a/primitives/core/src/lib.rs b/primitives/core/src/lib.rs index 36eca35d7fdfb..b7c8b69e8a0ab 100644 --- a/primitives/core/src/lib.rs +++ b/primitives/core/src/lib.rs @@ -48,11 +48,11 @@ pub use sp_debug_derive::RuntimeDebug; #[cfg(feature = "std")] pub use impl_serde::serialize as bytes; +#[cfg(feature = "full_crypto")] pub mod hashing; -pub use hashing::keccak_256; #[cfg(feature = "full_crypto")] -pub use hashing::{blake2_128, blake2_256, twox_128, twox_256, twox_64}; +pub use hashing::{blake2_128, blake2_256, keccak_256, twox_128, twox_256, twox_64}; pub mod crypto; pub mod hexdisplay; From f5feb46806359d844581d92bd20986f37a755a76 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Wed, 30 Mar 2022 16:25:51 +0300 Subject: [PATCH 08/26] ecdsa::Public(*pk).to_eth_address() moved to frame_support and contracts to use it --- Cargo.lock | 2 +- frame/contracts/src/exec.rs | 3 +- frame/support/Cargo.toml | 1 + frame/support/src/crypto.rs | 21 +++++++++ frame/support/src/crypto/ecdsa.rs | 76 +++++++++++++++++++++++++++++++ frame/support/src/lib.rs | 1 + primitives/core/Cargo.toml | 1 - primitives/core/src/ecdsa.rs | 28 ------------ 8 files changed, 102 insertions(+), 31 deletions(-) create mode 100644 frame/support/src/crypto.rs create mode 100644 frame/support/src/crypto/ecdsa.rs diff --git a/Cargo.lock b/Cargo.lock index 33c051adef4f6..32635bfb9427b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2261,6 +2261,7 @@ dependencies = [ "frame-support-procedural", "frame-system", "impl-trait-for-tuples", + "k256", "log 0.4.14", "once_cell", "parity-scale-codec", @@ -9917,7 +9918,6 @@ dependencies = [ "hex", "hex-literal", "impl-serde", - "k256", "lazy_static", "libsecp256k1", "log 0.4.14", diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index eefb2ca1f7e79..4356ea0648401 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -22,6 +22,7 @@ use crate::{ Pallet as Contracts, Schedule, }; use frame_support::{ + crypto::ecdsa, 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, ecdsa}; +use sp_core::crypto::UncheckedFrom; use sp_io::crypto::secp256k1_ecdsa_recover_compressed; use sp_runtime::traits::Convert; use sp_std::{marker::PhantomData, mem, prelude::*}; diff --git a/frame/support/Cargo.toml b/frame/support/Cargo.toml index 3161e4f4db69b..1fd68388d1a6b 100644 --- a/frame/support/Cargo.toml +++ b/frame/support/Cargo.toml @@ -35,6 +35,7 @@ impl-trait-for-tuples = "0.2.1" smallvec = "1.8.0" log = { version = "0.4.14", default-features = false } sp-core-hashing-proc-macro = { version = "5.0.0", path = "../../primitives/core/hashing/proc-macro" } +k256 = { version = "0.10.2", default-features = false, features = ["ecdsa"] } [dev-dependencies] assert_matches = "1.3.0" diff --git a/frame/support/src/crypto.rs b/frame/support/src/crypto.rs new file mode 100644 index 0000000000000..a12ca03680192 --- /dev/null +++ b/frame/support/src/crypto.rs @@ -0,0 +1,21 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Utilities for dealing with crypto primitives. Sometimes we need to use these from inside WASM +//! contracts, where crypto calculations have weak performance. + +pub mod ecdsa; diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs new file mode 100644 index 0000000000000..e60aef48ebb44 --- /dev/null +++ b/frame/support/src/crypto/ecdsa.rs @@ -0,0 +1,76 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Simple ECDSA secp256k1 API. Reduced version of such from sp_core to use in contracts. +use sp_core::{crypto::UncheckedFrom, ByteArray}; + +/// The ECDSA compressed public key. +pub struct Public(pub [u8; 33]); + +impl Public { + /// Converts self into Ethereum address + pub fn to_eth_address(&self) -> Result<[u8; 20], ()> { + use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; + + PublicKey::from_sec1_bytes(self.as_slice()) + .map(|pub_key| { + // uncompress the key + let uncompressed = pub_key.to_encoded_point(false); + // convert to ETH address + let res: [u8; 20] = sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..] + .try_into() + .unwrap(); + res + }) + .map_err(|_| ()) + } +} + +impl ByteArray for Public { + const LEN: usize = 33; +} + +impl AsRef<[u8]> for Public { + fn as_ref(&self) -> &[u8] { + &self.0[..] + } +} + +impl AsMut<[u8]> for Public { + fn as_mut(&mut self) -> &mut [u8] { + &mut self.0[..] + } +} + +impl sp_std::convert::TryFrom<&[u8]> for Public { + type Error = (); + + fn try_from(data: &[u8]) -> Result { + if data.len() != Self::LEN { + return Err(()) + } + let mut r = [0u8; Self::LEN]; + r.copy_from_slice(data); + Ok(Self::unchecked_from(r)) + } +} + +impl UncheckedFrom<[u8; 33]> for Public { + fn unchecked_from(x: [u8; 33]) -> Self { + Public(x) + } +} diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 70a79c07189dc..77aa29ce5e969 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -64,6 +64,7 @@ pub mod event; pub mod inherent; #[macro_use] pub mod error; +pub mod crypto; pub mod instances; pub mod migrations; pub mod traits; diff --git a/primitives/core/Cargo.toml b/primitives/core/Cargo.toml index 08601d3a7531a..413a11c9dc0a4 100644 --- a/primitives/core/Cargo.toml +++ b/primitives/core/Cargo.toml @@ -45,7 +45,6 @@ futures = { version = "0.3.19", optional = true } dyn-clonable = { version = "0.9.0", optional = true } thiserror = { version = "1.0.30", optional = true } bitflags = "1.3" -k256 = { version = "0.10.2", default-features = false, features = ["ecdsa"] } # full crypto ed25519-dalek = { version = "1.0.1", default-features = false, features = ["u64_backend", "alloc"], optional = true } diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index 3832ea21738f3..7a4e4399913dc 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -43,7 +43,6 @@ use secp256k1::{ ecdsa::{RecoverableSignature, RecoveryId}, Message, PublicKey, SecretKey, }; - #[cfg(feature = "std")] use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "full_crypto")] @@ -100,22 +99,6 @@ impl Public { }; pubkey.map(|k| Self(k.serialize())).map_err(|_| ()) } - - /// Converts self into Ethereum address - pub fn to_eth_address(&self) -> Result<[u8; 20], ()> { - use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; - - PublicKey::from_sec1_bytes(self.as_slice()) - .map(|pub_key| { - // uncompress the key - let uncompressed = pub_key.to_encoded_point(false); - // convert to ETH address - let res: [u8; 20] = - sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..].try_into().unwrap(); - res - }) - .map_err(|_| ()) - } } impl ByteArray for Public { @@ -880,15 +863,4 @@ mod test { let key = sig.recover_prehashed(&msg).unwrap(); assert_ne!(pair.public(), key); } - - #[test] - fn to_eth_address_works() { - let pair = Pair::from_string( - "0x9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60", - None, - ) - .unwrap(); - let eth_address = pair.public().to_eth_address(); - assert_eq!(eth_address.unwrap(), hex!("09231da7b19A016f9e576d23B16277062F4d46A8")[..]); - } } From b8019729b423058f5bbb3b3f4294f9379830409e Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Wed, 30 Mar 2022 18:16:33 +0300 Subject: [PATCH 09/26] beefy-mmr to use newly added frame_support function for convertion --- frame/beefy-mmr/src/lib.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index 17a9a04fd4bc8..757fbdfc0d9ac 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -40,7 +40,7 @@ use beefy_primitives::mmr::{BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion}; use pallet_mmr::primitives::LeafDataProvider; use codec::Encode; -use frame_support::traits::Get; +use frame_support::{crypto::ecdsa, traits::Get}; pub use pallet::*; @@ -72,11 +72,15 @@ where pub struct BeefyEcdsaToEthereum; impl Convert> for BeefyEcdsaToEthereum { fn convert(a: beefy_primitives::crypto::AuthorityId) -> Vec { - sp_core::ecdsa::Public::from(a) + ecdsa::Public::try_from(a.as_ref()) + .map_err(|_| { + log::error!(target: "runtime::beefy", "Invalid BEEFY PublicKey format!"); + }) + .unwrap() .to_eth_address() .map(|v| v.to_vec()) - .map_err(|| { - log::error!(target: "runtime::beefy", "Invalid BEEFY PublicKey format!"); + .map_err(|_| { + log::error!(target: "runtime::beefy", "Failed to convert BEEFY PublicKey to ETH address!"); }) .unwrap_or_default() } From be3e8a9791622cf8189d627ea37fa8e77dba817f Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Wed, 30 Mar 2022 18:33:43 +0300 Subject: [PATCH 10/26] a doc fix --- frame/support/src/crypto/ecdsa.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs index e60aef48ebb44..5e077061ab8ef 100644 --- a/frame/support/src/crypto/ecdsa.rs +++ b/frame/support/src/crypto/ecdsa.rs @@ -15,7 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Simple ECDSA secp256k1 API. Reduced version of such from sp_core to use in contracts. +//! Simple ECDSA secp256k1 API. This is a reduced version of sp_core::crypto::ecdsa to use in +//! contracts. use sp_core::{crypto::UncheckedFrom, ByteArray}; /// The ECDSA compressed public key. From 4037170eb37f08337ddba07cf3392c0a94ab5b6f Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Wed, 30 Mar 2022 18:57:50 +0300 Subject: [PATCH 11/26] import fix --- frame/support/src/crypto/ecdsa.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs index 5e077061ab8ef..3717051670fd3 100644 --- a/frame/support/src/crypto/ecdsa.rs +++ b/frame/support/src/crypto/ecdsa.rs @@ -17,7 +17,7 @@ //! Simple ECDSA secp256k1 API. This is a reduced version of sp_core::crypto::ecdsa to use in //! contracts. -use sp_core::{crypto::UncheckedFrom, ByteArray}; +use sp_core::crypto::{ByteArray, UncheckedFrom}; /// The ECDSA compressed public key. pub struct Public(pub [u8; 33]); From c8ff0efd98f72c1449c583d5ffcc1d2d081e3127 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Wed, 30 Mar 2022 20:23:18 +0300 Subject: [PATCH 12/26] benchmark fix-1 (still fails) --- frame/contracts/src/benchmarking/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 31ccef29d6998..87df094d4e730 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -1989,9 +1989,9 @@ benchmarks! { }, ], call_body: Some(body::repeated_dyn(r * API_BENCHMARK_BATCH_SIZE, vec![ - Counter(20, 33), // pub_key_ptr - Regular(Instruction::I32Const(0)), // out_ptr - Regular(Instruction::I32Const(4)), // out_len_ptr + Counter(24, 33), // pub_key_ptr + Regular(Instruction::I32Const(4)), // out_ptr + Regular(Instruction::I32Const(0)), // out_len_ptr Regular(Instruction::Call(0)), Regular(Instruction::Drop), ])), From 976ed7e1d6768f0df88e0507037170b3ddbe9690 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Thu, 31 Mar 2022 14:28:13 +0300 Subject: [PATCH 13/26] benchmark fixed --- frame/contracts/src/benchmarking/mod.rs | 19 +++++++++---------- frame/contracts/src/wasm/mod.rs | 15 ++++----------- frame/contracts/src/wasm/runtime.rs | 24 ++++++++++++++---------- 3 files changed, 27 insertions(+), 31 deletions(-) diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 87df094d4e730..ab019d0941531 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -1968,32 +1968,31 @@ benchmarks! { seal_ecdsa_to_eth_address { let r in 0 .. API_BENCHMARK_BATCHES; let key_type = sp_core::crypto::KeyTypeId(*b"code"); - let pub_keys = (0..r * API_BENCHMARK_BATCH_SIZE) + let pub_keys_bytes = (0..r * API_BENCHMARK_BATCH_SIZE) .map(|_| { - sp_io::crypto::ecdsa_generate(key_type, None).encode() + sp_io::crypto::ecdsa_generate(key_type, None).0 }) - .collect::>(); - let pub_keys_bytes = pub_keys.iter().map(|a| a.encode()).flatten().collect::>(); + .flatten() + .collect::>(); + let pub_keys_bytes_len = pub_keys_bytes.len() as i32; let code = WasmModule::::from(ModuleDefinition { memory: Some(ImportedMemory::max::()), imported_functions: vec![ImportedFunction { module: "__unstable__", name: "seal_ecdsa_to_eth_address", - params: vec![ValueType::I32, ValueType::I32, ValueType::I32], + params: vec![ValueType::I32, ValueType::I32], return_type: None, }], data_segments: vec![ DataSegment { - offset: 24, + offset: 0, value: pub_keys_bytes, }, ], call_body: Some(body::repeated_dyn(r * API_BENCHMARK_BATCH_SIZE, vec![ - Counter(24, 33), // pub_key_ptr - Regular(Instruction::I32Const(4)), // out_ptr - Regular(Instruction::I32Const(0)), // out_len_ptr + Counter(0, 33), // pub_key_ptr + Regular(Instruction::I32Const(pub_keys_bytes_len)), // out_ptr Regular(Instruction::Call(0)), - Regular(Instruction::Drop), ])), .. Default::default() }); diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index d29ac64b0e8ee..ec6ea13148fbf 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -1095,11 +1095,11 @@ mod tests { /// 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 i32))) + (import "__unstable__" "seal_ecdsa_to_eth_address" (func $seal_ecdsa_to_eth_address (param i32 i32))) (import "env" "memory" (memory 1 1)) - ;; size of our buffer is 20 bytes - (data (i32.const 20) "\20") + ;; size of our buffer is 33 bytes + (data (i32.const 33) "\20") (func $assert (param i32) (block $ok @@ -1112,15 +1112,8 @@ mod tests { (func (export "call") ;; fill the buffer with the eth address. - (call $seal_ecdsa_to_eth_address (i32.const 0) (i32.const 0) (i32.const 20)) + (call $seal_ecdsa_to_eth_address (i32.const 0) (i32.const 0)) - ;; assert out_len == 20 - (call $assert - (i32.eq - (i32.load (i32.const 20)) - (i32.const 20) - ) - ) ;; assert that the mock returned 20 zero-bytes (call $assert (i64.eq diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 6b700a76dec74..d209666cf8015 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -2041,7 +2041,7 @@ define_env!(Env, , // // # Parameters // - // - code_hash_ptr: A pointer to the buffer that contains the new code hash. + // - `code_hash_ptr`: A pointer to the buffer that contains the new code hash. // // # Errors // @@ -2063,17 +2063,21 @@ define_env!(Env, , // // # Parameters // - // - key_ptr: a pointer to the ECDSA compressed public key + // - `key_ptr`: a pointer to the ECDSA compressed public key. Should be decodable as a 33 bytes value. + // Traps otherwise. + // - `out_ptr`: the pointer into the linear memory where the output + // data is placed. // // The value is stored to linear memory at the address pointed to by `out_ptr`. - // `out_len_ptr` must point to a u32 value that describes the available space at - // `out_ptr`. This call overwrites it with the size of the value. If the available - // space at `out_ptr` is less than the size of the value a trap is triggered. - [__unstable__] seal_ecdsa_to_eth_address(ctx, key_ptr: u32, out_ptr: u32, out_len_ptr: u32) => { + // If the available space at `out_ptr` is less than the size of the value a trap is triggered. + [__unstable__] seal_ecdsa_to_eth_address(ctx, key_ptr: u32, out_ptr: u32) => { ctx.charge_gas(RuntimeCosts::EcdsaToEthAddress)?; - let compressed_key: [u8; 33] = - ctx.read_sandbox_memory_as(key_ptr)?; - let result = ctx.ext.ecdsa_to_eth_address(&compressed_key).unwrap(); - Ok(ctx.write_sandbox_output(out_ptr, out_len_ptr, &result, false, already_charged)?) + let mut compressed_key: [u8; 33] = [0;33]; + ctx.read_sandbox_memory_into_buf(key_ptr, &mut compressed_key)?; + let result = ctx.ext.ecdsa_to_eth_address(&compressed_key); + match result { + Ok(eth_address) => Ok(ctx.write_sandbox_memory(out_ptr, eth_address.as_ref())?), + Err(_) => Err(TrapReason::SupervisorError(DispatchError::Other("Failed to convert ECDSA compressed public key into ETH address!")))? + } }, ); From ec8d860b891be645d68e83714ca849a42a8e357c Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Fri, 8 Apr 2022 20:40:37 +0300 Subject: [PATCH 14/26] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexander Theißen --- frame/contracts/src/wasm/runtime.rs | 2 +- frame/support/src/crypto/ecdsa.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index d209666cf8015..fbfcd3712658f 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -2059,7 +2059,7 @@ define_env!(Env, , }, // Calculates Ethereum address from the ECDSA compressed public key and stores - // it into the supplied buffer + // it into the supplied buffer. // // # Parameters // diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs index 3717051670fd3..04e880bdb531b 100644 --- a/frame/support/src/crypto/ecdsa.rs +++ b/frame/support/src/crypto/ecdsa.rs @@ -15,8 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Simple ECDSA secp256k1 API. This is a reduced version of sp_core::crypto::ecdsa to use in -//! contracts. +//! Simple ECDSA secp256k1 API. This is a reduced version of `sp_core::crypto::ecdsa` for use in +//! cases where the performance penalty of doing in-runtime crypto isn't severe. In case this +//! becomes a performance bottleneck a new host function should be considered. use sp_core::crypto::{ByteArray, UncheckedFrom}; /// The ECDSA compressed public key. From b5be45d17225459ecfeeac24df8b3d8c9429f250 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Fri, 8 Apr 2022 20:37:45 +0300 Subject: [PATCH 15/26] fixes on Alex T feedback --- frame/contracts/src/wasm/mod.rs | 36 ++++++++++++++--------------- frame/contracts/src/wasm/runtime.rs | 9 +++++--- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index ec6ea13148fbf..70323d64fbc2c 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -504,7 +504,7 @@ mod tests { unimplemented!() } fn ecdsa_to_eth_address(&self, _pk: &[u8; 33]) -> Result<[u8; 20], ()> { - Ok([0u8; 20]) + Ok([2u8; 20]) } } @@ -1095,38 +1095,36 @@ mod tests { /// 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))) + (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)) ;; size of our buffer is 33 bytes - (data (i32.const 33) "\20") - - (func $assert (param i32) - (block $ok - (br_if $ok - (get_local 0) - ) - (unreachable) - ) - ) + (data (i32.const 0) "\21") (func (export "call") ;; fill the buffer with the eth address. (call $seal_ecdsa_to_eth_address (i32.const 0) (i32.const 0)) - ;; assert that the mock returned 20 zero-bytes - (call $assert - (i64.eq - (i64.load (i32.const 0)) - (i64.const 0x000000000000000000000000000000000000000) - ) + ;; Return the contents of the buffer + (call $seal_return + (i32.const 0) + (i32.const 0) + (i32.const 20) ) + + ;; env:seal_return doesn't return, so this is effectively unreachable. + (unreachable) ) (func (export "deploy")) ) "#; - assert_ok!(execute(CODE_ECDSA_TO_ETH_ADDRESS, vec![], MockExt::default())); + 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#" diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index fbfcd3712658f..e814496f57669 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -2070,14 +2070,17 @@ define_env!(Env, , // // The value is stored to linear memory at the address pointed to by `out_ptr`. // If the available space at `out_ptr` is less than the size of the value a trap is triggered. - [__unstable__] seal_ecdsa_to_eth_address(ctx, key_ptr: u32, out_ptr: u32) => { + [__unstable__] seal_ecdsa_to_eth_address(ctx, key_ptr: u32, out_ptr: u32) -> ReturnCode => { ctx.charge_gas(RuntimeCosts::EcdsaToEthAddress)?; let mut compressed_key: [u8; 33] = [0;33]; ctx.read_sandbox_memory_into_buf(key_ptr, &mut compressed_key)?; let result = ctx.ext.ecdsa_to_eth_address(&compressed_key); match result { - Ok(eth_address) => Ok(ctx.write_sandbox_memory(out_ptr, eth_address.as_ref())?), - Err(_) => Err(TrapReason::SupervisorError(DispatchError::Other("Failed to convert ECDSA compressed public key into ETH address!")))? + Ok(eth_address) => { + ctx.write_sandbox_memory(out_ptr, eth_address.as_ref())?; + Ok(ReturnCode::Success) + }, + Err(_) => Ok(ReturnCode::EcdsaRecoverFailed), } }, ); From 877b57e4de6793ec9dc4f8cfc4661362018404a1 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Fri, 8 Apr 2022 22:23:16 +0300 Subject: [PATCH 16/26] to_eth_address() put into extension trait for sp-core::ecdsa::Public --- frame/beefy-mmr/src/lib.rs | 4 +-- frame/contracts/src/exec.rs | 6 ++-- frame/support/src/crypto/ecdsa.rs | 49 +++++-------------------------- 3 files changed, 13 insertions(+), 46 deletions(-) diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index 0760613fa2126..4aad15f0b0b97 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -39,7 +39,7 @@ use sp_std::prelude::*; use beefy_primitives::mmr::{BeefyDataProvider, BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion}; use pallet_mmr::primitives::LeafDataProvider; -use frame_support::{crypto::ecdsa, traits::Get}; +use frame_support::{crypto::ecdsa::RuntimeECDSA, traits::Get}; pub use pallet::*; @@ -71,7 +71,7 @@ where pub struct BeefyEcdsaToEthereum; impl Convert> for BeefyEcdsaToEthereum { fn convert(a: beefy_primitives::crypto::AuthorityId) -> Vec { - ecdsa::Public::try_from(a.as_ref()) + sp_core::ecdsa::Public::try_from(a.as_ref()) .map_err(|_| { log::error!(target: "runtime::beefy", "Invalid BEEFY PublicKey format!"); }) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 9f9bd88dd0a2a..8d69051907009 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -22,7 +22,7 @@ use crate::{ Pallet as Contracts, Schedule, }; use frame_support::{ - crypto::ecdsa, + crypto::ecdsa::RuntimeECDSA, dispatch::{DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable}, storage::{with_transaction, TransactionOutcome}, traits::{Contains, Currency, ExistenceRequirement, OriginTrait, Randomness, Time}, @@ -31,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::*}; @@ -1209,7 +1209,7 @@ where } fn ecdsa_to_eth_address(&self, pk: &[u8; 33]) -> Result<[u8; 20], ()> { - ecdsa::Public(*pk).to_eth_address() + ECDSAPublic(*pk).to_eth_address() } #[cfg(test)] diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs index 04e880bdb531b..1dd1694b1c954 100644 --- a/frame/support/src/crypto/ecdsa.rs +++ b/frame/support/src/crypto/ecdsa.rs @@ -18,14 +18,16 @@ //! Simple ECDSA secp256k1 API. This is a reduced version of `sp_core::crypto::ecdsa` for use in //! cases where the performance penalty of doing in-runtime crypto isn't severe. In case this //! becomes a performance bottleneck a new host function should be considered. -use sp_core::crypto::{ByteArray, UncheckedFrom}; +use sp_core::{crypto::ByteArray, ecdsa::Public}; -/// The ECDSA compressed public key. -pub struct Public(pub [u8; 33]); +/// Extension trait for `sp_core::ecdsa::Public` to be used from inside runtime. +pub trait RuntimeECDSA { + /// Returns Ethereum address calculated from this ECDSA public key. + fn to_eth_address(&self) -> Result<[u8; 20], ()>; +} -impl Public { - /// Converts self into Ethereum address - pub fn to_eth_address(&self) -> Result<[u8; 20], ()> { +impl RuntimeECDSA for Public { + fn to_eth_address(&self) -> Result<[u8; 20], ()> { use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; PublicKey::from_sec1_bytes(self.as_slice()) @@ -41,38 +43,3 @@ impl Public { .map_err(|_| ()) } } - -impl ByteArray for Public { - const LEN: usize = 33; -} - -impl AsRef<[u8]> for Public { - fn as_ref(&self) -> &[u8] { - &self.0[..] - } -} - -impl AsMut<[u8]> for Public { - fn as_mut(&mut self) -> &mut [u8] { - &mut self.0[..] - } -} - -impl sp_std::convert::TryFrom<&[u8]> for Public { - type Error = (); - - fn try_from(data: &[u8]) -> Result { - if data.len() != Self::LEN { - return Err(()) - } - let mut r = [0u8; Self::LEN]; - r.copy_from_slice(data); - Ok(Self::unchecked_from(r)) - } -} - -impl UncheckedFrom<[u8; 33]> for Public { - fn unchecked_from(x: [u8; 33]) -> Self { - Public(x) - } -} From b1ee31fa03d251a77db44b9f156bd8a0b009a6fb Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Mon, 11 Apr 2022 17:14:00 +0300 Subject: [PATCH 17/26] Update frame/support/src/crypto/ecdsa.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexander Theißen --- frame/support/src/crypto/ecdsa.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs index 1dd1694b1c954..d1a7e1c9a8c5f 100644 --- a/frame/support/src/crypto/ecdsa.rs +++ b/frame/support/src/crypto/ecdsa.rs @@ -20,7 +20,12 @@ //! becomes a performance bottleneck a new host function should be considered. use sp_core::{crypto::ByteArray, ecdsa::Public}; -/// Extension trait for `sp_core::ecdsa::Public` to be used from inside runtime. +/// Extension trait for [`Public`] to be used from inside the runtime. +/// +/// # Note +/// +/// This is needed because host functions cannot be called from within +/// `sp_core` due to cyclic dependencies on `sp_io`. pub trait RuntimeECDSA { /// Returns Ethereum address calculated from this ECDSA public key. fn to_eth_address(&self) -> Result<[u8; 20], ()>; From 16fde7107785d9b6b72caecbaa8deb60b218fbe3 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Mon, 11 Apr 2022 17:15:58 +0300 Subject: [PATCH 18/26] Update frame/contracts/src/wasm/mod.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexander Theißen --- frame/contracts/src/wasm/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 70323d64fbc2c..36038b2a39d86 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -1113,7 +1113,7 @@ mod tests { (i32.const 20) ) - ;; env:seal_return doesn't return, so this is effectively unreachable. + ;; seal_return doesn't return, so this is effectively unreachable. (unreachable) ) (func (export "deploy")) From cdd6bdb2322f6824f4f5954c6cc02049de1ea20c Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Mon, 11 Apr 2022 21:03:14 +0300 Subject: [PATCH 19/26] fixes on issues pointed out in review --- frame/contracts/src/wasm/mod.rs | 3 --- frame/contracts/src/wasm/runtime.rs | 13 +++++++------ frame/support/src/crypto/ecdsa.rs | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 36038b2a39d86..f12df06b938c8 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -1099,9 +1099,6 @@ mod tests { (import "seal0" "seal_return" (func $seal_return (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) - ;; size of our buffer is 33 bytes - (data (i32.const 0) "\21") - (func (export "call") ;; fill the buffer with the eth address. (call $seal_ecdsa_to_eth_address (i32.const 0) (i32.const 0)) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index e814496f57669..b4106b07ea8cc 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -1989,12 +1989,12 @@ define_env!(Env, , // # Parameters // // - `signature_ptr`: the pointer into the linear memory where the signature - // is placed. Should be decodable as a 65 bytes. Traps otherwise. + // is placed. Should be decodable as a 65 bytes. Traps otherwise. // - `message_hash_ptr`: the pointer into the linear memory where the message // hash is placed. Should be decodable as a 32 bytes. Traps otherwise. // - `output_ptr`: the pointer into the linear memory where the output - // data is placed. The buffer should be 33 bytes. Traps otherwise. - // The function will write the result directly into this buffer. + // data is placed. The buffer should be 33 bytes. The function + // will write the result directly into this buffer. // // # Errors // @@ -2066,7 +2066,8 @@ define_env!(Env, , // - `key_ptr`: a pointer to the ECDSA compressed public key. Should be decodable as a 33 bytes value. // Traps otherwise. // - `out_ptr`: the pointer into the linear memory where the output - // data is placed. + // data is placed. The function will write the result + // directly into this buffer. // // The value is stored to linear memory at the address pointed to by `out_ptr`. // If the available space at `out_ptr` is less than the size of the value a trap is triggered. @@ -2077,8 +2078,8 @@ define_env!(Env, , let result = ctx.ext.ecdsa_to_eth_address(&compressed_key); match result { Ok(eth_address) => { - ctx.write_sandbox_memory(out_ptr, eth_address.as_ref())?; - Ok(ReturnCode::Success) + ctx.write_sandbox_memory(out_ptr, eth_address.as_ref())?; + Ok(ReturnCode::Success) }, Err(_) => Ok(ReturnCode::EcdsaRecoverFailed), } diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs index d1a7e1c9a8c5f..67e2991fade80 100644 --- a/frame/support/src/crypto/ecdsa.rs +++ b/frame/support/src/crypto/ecdsa.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Simple ECDSA secp256k1 API. This is a reduced version of `sp_core::crypto::ecdsa` for use in +//! Simple ECDSA secp256k1 API. This is an extension trait for `sp_core::ecdsa::Public` to be used in //! cases where the performance penalty of doing in-runtime crypto isn't severe. In case this //! becomes a performance bottleneck a new host function should be considered. use sp_core::{crypto::ByteArray, ecdsa::Public}; From b51a7b0968d16046a4dad06e7c6ec955472f068d Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Mon, 11 Apr 2022 22:00:02 +0300 Subject: [PATCH 20/26] benchmark errors fixed --- frame/contracts/src/benchmarking/mod.rs | 3 ++- frame/contracts/src/wasm/runtime.rs | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index ab019d0941531..e67436fbc9d37 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -1981,7 +1981,7 @@ benchmarks! { module: "__unstable__", name: "seal_ecdsa_to_eth_address", params: vec![ValueType::I32, ValueType::I32], - return_type: None, + return_type: Some(ValueType::I32), }], data_segments: vec![ DataSegment { @@ -1993,6 +1993,7 @@ benchmarks! { Counter(0, 33), // pub_key_ptr Regular(Instruction::I32Const(pub_keys_bytes_len)), // out_ptr Regular(Instruction::Call(0)), + Regular(Instruction::Drop), ])), .. Default::default() }); diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index b4106b07ea8cc..905117134082b 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -2071,6 +2071,10 @@ define_env!(Env, , // // The value is stored to linear memory at the address pointed to by `out_ptr`. // If the available space at `out_ptr` is less than the size of the value a trap is triggered. + // + // # Errors + // + // `ReturnCode::EcdsaRecoverFailed` [__unstable__] seal_ecdsa_to_eth_address(ctx, key_ptr: u32, out_ptr: u32) -> ReturnCode => { ctx.charge_gas(RuntimeCosts::EcdsaToEthAddress)?; let mut compressed_key: [u8; 33] = [0;33]; From 41ba268523437f27ce2e2f6bac4bca4b026ff96f Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Mon, 11 Apr 2022 22:12:34 +0300 Subject: [PATCH 21/26] fmt fix --- frame/support/src/crypto/ecdsa.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs index 67e2991fade80..838a2f1f7f171 100644 --- a/frame/support/src/crypto/ecdsa.rs +++ b/frame/support/src/crypto/ecdsa.rs @@ -15,8 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Simple ECDSA secp256k1 API. This is an extension trait for `sp_core::ecdsa::Public` to be used in -//! cases where the performance penalty of doing in-runtime crypto isn't severe. In case this +//! Simple ECDSA secp256k1 API. This is an extension trait for `sp_core::ecdsa::Public` to be used +//! in cases where the performance penalty of doing in-runtime crypto isn't severe. In case this //! becomes a performance bottleneck a new host function should be considered. use sp_core::{crypto::ByteArray, ecdsa::Public}; From 34779f8f2b1959e9c3fd0cc424fbf07234005647 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Mon, 11 Apr 2022 22:53:01 +0300 Subject: [PATCH 22/26] EcdsaRecoverFailed err docs updated --- frame/contracts/src/wasm/runtime.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 905117134082b..876bd8848b0b1 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -71,7 +71,9 @@ pub enum ReturnCode { /// The call dispatched by `seal_call_runtime` was executed but returned an error. #[cfg(feature = "unstable-interface")] CallRuntimeReturnedError = 10, - /// ECDSA pubkey recovery failed. Most probably wrong recovery id or signature. + /// ECDSA pubkey recovery failed (most probably wrong recovery id or signature), or + /// ECDSA compressed pubkey conversion into Ethereum address failed (most probably + /// wrong pubkey provided). #[cfg(feature = "unstable-interface")] EcdsaRecoverFailed = 11, } From 7c5fa913b513be698c932192fd451739fbbc9160 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Thu, 14 Apr 2022 19:59:13 +0300 Subject: [PATCH 23/26] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/support/src/crypto/ecdsa.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs index 838a2f1f7f171..ebee451900f85 100644 --- a/frame/support/src/crypto/ecdsa.rs +++ b/frame/support/src/crypto/ecdsa.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// Copyright (C) 2022 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -15,9 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Simple ECDSA secp256k1 API. This is an extension trait for `sp_core::ecdsa::Public` to be used -//! in cases where the performance penalty of doing in-runtime crypto isn't severe. In case this -//! becomes a performance bottleneck a new host function should be considered. +//! Simple ECDSA secp256k1 API. +//! +//! Provides an extension trait for [`sp_core::ecdsa::Public`] to do certain operations. + use sp_core::{crypto::ByteArray, ecdsa::Public}; /// Extension trait for [`Public`] to be used from inside the runtime. @@ -26,24 +27,22 @@ use sp_core::{crypto::ByteArray, ecdsa::Public}; /// /// This is needed because host functions cannot be called from within /// `sp_core` due to cyclic dependencies on `sp_io`. -pub trait RuntimeECDSA { +pub trait ECDSAExt { /// Returns Ethereum address calculated from this ECDSA public key. fn to_eth_address(&self) -> Result<[u8; 20], ()>; } -impl RuntimeECDSA for Public { +impl ECDSAExt for Public { fn to_eth_address(&self) -> Result<[u8; 20], ()> { use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; PublicKey::from_sec1_bytes(self.as_slice()) - .map(|pub_key| { + .and_then(|pub_key| { // uncompress the key let uncompressed = pub_key.to_encoded_point(false); // convert to ETH address - let res: [u8; 20] = sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..] - .try_into() - .unwrap(); - res + [u8; 20]::try_from(sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..]) + .map_err(drop) }) .map_err(|_| ()) } From ec3a2af032f3b79182a5c03b0fb7e9d346aa9a8a Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Thu, 14 Apr 2022 23:05:24 +0300 Subject: [PATCH 24/26] make applied suggestions compile --- frame/beefy-mmr/src/lib.rs | 2 +- frame/contracts/src/exec.rs | 2 +- frame/support/src/crypto/ecdsa.rs | 14 ++++++++------ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index 4aad15f0b0b97..64d32b6b8182f 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -39,7 +39,7 @@ use sp_std::prelude::*; use beefy_primitives::mmr::{BeefyDataProvider, BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion}; use pallet_mmr::primitives::LeafDataProvider; -use frame_support::{crypto::ecdsa::RuntimeECDSA, traits::Get}; +use frame_support::{crypto::ecdsa::ECDSAExt, traits::Get}; pub use pallet::*; diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 8d69051907009..54a5223b53d21 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -22,7 +22,7 @@ use crate::{ Pallet as Contracts, Schedule, }; use frame_support::{ - crypto::ecdsa::RuntimeECDSA, + crypto::ecdsa::ECDSAExt, dispatch::{DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable}, storage::{with_transaction, TransactionOutcome}, traits::{Contains, Currency, ExistenceRequirement, OriginTrait, Randomness, Time}, diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs index ebee451900f85..c1ac438cf40a9 100644 --- a/frame/support/src/crypto/ecdsa.rs +++ b/frame/support/src/crypto/ecdsa.rs @@ -15,9 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Simple ECDSA secp256k1 API. +//! Simple ECDSA secp256k1 API. //! -//! Provides an extension trait for [`sp_core::ecdsa::Public`] to do certain operations. +//! Provides an extension trait for [`sp_core::ecdsa::Public`] to do certain operations. use sp_core::{crypto::ByteArray, ecdsa::Public}; @@ -37,13 +37,15 @@ impl ECDSAExt for Public { use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; PublicKey::from_sec1_bytes(self.as_slice()) - .and_then(|pub_key| { + .map(|pub_key| { // uncompress the key let uncompressed = pub_key.to_encoded_point(false); // convert to ETH address - [u8; 20]::try_from(sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..]) - .map_err(drop) + <[u8; 20]>::try_from( + sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..].as_ref(), + ) + .expect("failed to take last 20 bytes of a keccak_256 hash") }) - .map_err(|_| ()) + .map_err(drop) } } From dffce61068e7734e0c8009597288900fb32d07f0 Mon Sep 17 00:00:00 2001 From: Alexander Gryaznov Date: Thu, 14 Apr 2022 23:22:12 +0300 Subject: [PATCH 25/26] get rid of unwrap() in runtime --- frame/beefy-mmr/src/lib.rs | 2 +- frame/support/src/crypto.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index 64d32b6b8182f..b58433a304583 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -75,7 +75,7 @@ impl Convert> for BeefyEcdsaToEth .map_err(|_| { log::error!(target: "runtime::beefy", "Invalid BEEFY PublicKey format!"); }) - .unwrap() + .unwrap_or(sp_core::ecdsa::Public::from_raw([0u8; 33])) .to_eth_address() .map(|v| v.to_vec()) .map_err(|_| { diff --git a/frame/support/src/crypto.rs b/frame/support/src/crypto.rs index a12ca03680192..182a784649d02 100644 --- a/frame/support/src/crypto.rs +++ b/frame/support/src/crypto.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// Copyright (C) 2022 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); From 2997b7ea623bf19c967b8c90f8fc6574b55c0236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sat, 16 Apr 2022 12:22:30 +0200 Subject: [PATCH 26/26] Remove expect --- frame/support/src/crypto/ecdsa.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/frame/support/src/crypto/ecdsa.rs b/frame/support/src/crypto/ecdsa.rs index c1ac438cf40a9..a4a04acabe1df 100644 --- a/frame/support/src/crypto/ecdsa.rs +++ b/frame/support/src/crypto/ecdsa.rs @@ -36,16 +36,14 @@ impl ECDSAExt for Public { fn to_eth_address(&self) -> Result<[u8; 20], ()> { use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; - PublicKey::from_sec1_bytes(self.as_slice()) - .map(|pub_key| { - // uncompress the key - let uncompressed = pub_key.to_encoded_point(false); - // convert to ETH address - <[u8; 20]>::try_from( - sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..].as_ref(), - ) - .expect("failed to take last 20 bytes of a keccak_256 hash") - }) + PublicKey::from_sec1_bytes(self.as_slice()).map_err(drop).and_then(|pub_key| { + // uncompress the key + let uncompressed = pub_key.to_encoded_point(false); + // convert to ETH address + <[u8; 20]>::try_from( + sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..].as_ref(), + ) .map_err(drop) + }) } }