From 385446fe083882c726fe912233fe63741b39359e Mon Sep 17 00:00:00 2001 From: librelois Date: Wed, 30 Nov 2022 11:27:07 +0100 Subject: [PATCH] ed25519_verify: Support using dalek for historical blocks (#12661) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ed25519_verify: Support using dalek for historical blocks The switch from `ed25519-dalek` to `ed25519-zebra` was actually a breaking change. `ed25519-zebra` is more permissive. To support historical blocks when syncing a chain this pull request introduces an externalities extension `UseDalekExt`. This extension is just used as a signaling mechanism to `ed25519_verify` to use `ed25519-dalek` when it is present. Together with `ExtensionBeforeBlock` it can be used to setup a node in way to sync historical blocks that require `ed25519-dalek`, because they included a transaction that verified differently as when using `ed25519-zebra`. This feature can be enabled in the following way. In the chain service file, directly after the client is created, the following code should be added: ``` use sc_client_api::ExecutorProvider; client.execution_extensions().set_extensions_factory( sc_client_api::execution_extensions::ExtensionBeforeBlock::::new(BLOCK_NUMBER_UNTIL_DALEK_SHOULD_BE_USED) ); ``` * Fix doc * More fixes * Update client/api/src/execution_extensions.rs Co-authored-by: AndrĂ© Silva <123550+andresilva@users.noreply.github.com> * Fix merge and warning * Fix docs Co-authored-by: AndrĂ© Silva <123550+andresilva@users.noreply.github.com> --- Cargo.lock | 1 + client/api/src/call_executor.rs | 23 ++-- client/api/src/execution_extensions.rs | 128 ++++++++++++++++---- client/finality-grandpa/src/lib.rs | 1 - client/rpc/src/state/state_full.rs | 1 - client/service/src/builder.rs | 2 +- client/service/src/client/call_executor.rs | 65 ++++++---- client/service/src/client/client.rs | 35 +++--- client/service/test/src/client/mod.rs | 40 +++++- client/tracing/src/lib.rs | 6 +- primitives/application-crypto/src/traits.rs | 4 +- primitives/externalities/src/extensions.rs | 22 +++- primitives/io/Cargo.toml | 2 + primitives/io/src/lib.rs | 69 ++++++++++- primitives/state-machine/src/lib.rs | 4 +- test-utils/client/src/lib.rs | 10 +- test-utils/runtime/src/lib.rs | 12 ++ 17 files changed, 324 insertions(+), 101 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7264997d31538..b48274628d3f5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9683,6 +9683,7 @@ name = "sp-io" version = "6.0.0" dependencies = [ "bytes", + "ed25519-dalek", "futures", "hash-db", "libsecp256k1", diff --git a/client/api/src/call_executor.rs b/client/api/src/call_executor.rs index 949fd16a30704..7a42385010c68 100644 --- a/client/api/src/call_executor.rs +++ b/client/api/src/call_executor.rs @@ -19,13 +19,12 @@ //! A method call executor interface. use sc_executor::{RuntimeVersion, RuntimeVersionOf}; -use sp_externalities::Extensions; use sp_runtime::{generic::BlockId, traits::Block as BlockT}; -use sp_state_machine::{ExecutionManager, ExecutionStrategy, OverlayedChanges, StorageProof}; +use sp_state_machine::{ExecutionStrategy, OverlayedChanges, StorageProof}; use std::cell::RefCell; use crate::execution_extensions::ExecutionExtensions; -use sp_api::{ProofRecorder, StorageTransactionCache}; +use sp_api::{ExecutionContext, ProofRecorder, StorageTransactionCache}; /// Executor Provider pub trait ExecutorProvider { @@ -47,6 +46,9 @@ pub trait CallExecutor: RuntimeVersionOf { /// The backend used by the node. type Backend: crate::backend::Backend; + /// Returns the [`ExecutionExtensions`]. + fn execution_extensions(&self) -> &ExecutionExtensions; + /// Execute a call to a contract on top of state in a block of given hash. /// /// No changes are made. @@ -56,7 +58,6 @@ pub trait CallExecutor: RuntimeVersionOf { method: &str, call_data: &[u8], strategy: ExecutionStrategy, - extensions: Option, ) -> Result, sp_blockchain::Error>; /// Execute a contextual call on top of state in a block of a given hash. @@ -64,12 +65,7 @@ pub trait CallExecutor: RuntimeVersionOf { /// No changes are made. /// Before executing the method, passed header is installed as the current header /// of the execution context. - fn contextual_call< - EM: Fn( - Result, Self::Error>, - Result, Self::Error>, - ) -> Result, Self::Error>, - >( + fn contextual_call( &self, at: &BlockId, method: &str, @@ -80,12 +76,9 @@ pub trait CallExecutor: RuntimeVersionOf { StorageTransactionCache>::State>, >, >, - execution_manager: ExecutionManager, proof_recorder: &Option>, - extensions: Option, - ) -> sp_blockchain::Result> - where - ExecutionManager: Clone; + context: ExecutionContext, + ) -> sp_blockchain::Result>; /// Extract RuntimeVersion of given block /// diff --git a/client/api/src/execution_extensions.rs b/client/api/src/execution_extensions.rs index 07a483bc3eaf2..58c085a29a945 100644 --- a/client/api/src/execution_extensions.rs +++ b/client/api/src/execution_extensions.rs @@ -29,12 +29,18 @@ use sp_core::{ offchain::{self, OffchainDbExt, OffchainWorkerExt, TransactionPoolExt}, ExecutionContext, }; -use sp_externalities::Extensions; +use sp_externalities::{Extension, Extensions}; use sp_keystore::{KeystoreExt, SyncCryptoStorePtr}; -use sp_runtime::{generic::BlockId, traits}; +use sp_runtime::{ + generic::BlockId, + traits::{Block as BlockT, NumberFor}, +}; pub use sp_state_machine::ExecutionStrategy; use sp_state_machine::{DefaultHandler, ExecutionManager}; -use std::sync::{Arc, Weak}; +use std::{ + marker::PhantomData, + sync::{Arc, Weak}, +}; /// Execution strategies settings. #[derive(Debug, Clone)] @@ -63,18 +69,81 @@ impl Default for ExecutionStrategies { } } -/// Generate the starting set of ExternalitiesExtensions based upon the given capabilities -pub trait ExtensionsFactory: Send + Sync { - /// Make `Extensions` for given `Capabilities`. - fn extensions_for(&self, capabilities: offchain::Capabilities) -> Extensions; +/// Generate the starting set of [`Extensions`]. +/// +/// These [`Extensions`] are passed to the environment a runtime is executed in. +pub trait ExtensionsFactory: Send + Sync { + /// Create [`Extensions`] for the given input. + /// + /// - `block_hash`: The hash of the block in the context that extensions will be used. + /// - `block_number`: The number of the block in the context that extensions will be used. + /// - `capabilities`: The capabilities + fn extensions_for( + &self, + block_hash: Block::Hash, + block_number: NumberFor, + capabilities: offchain::Capabilities, + ) -> Extensions; } -impl ExtensionsFactory for () { - fn extensions_for(&self, _capabilities: offchain::Capabilities) -> Extensions { +impl ExtensionsFactory for () { + fn extensions_for( + &self, + _: Block::Hash, + _: NumberFor, + _capabilities: offchain::Capabilities, + ) -> Extensions { Extensions::new() } } +impl> ExtensionsFactory for Vec { + fn extensions_for( + &self, + block_hash: Block::Hash, + block_number: NumberFor, + capabilities: offchain::Capabilities, + ) -> Extensions { + let mut exts = Extensions::new(); + exts.extend(self.iter().map(|e| e.extensions_for(block_hash, block_number, capabilities))); + exts + } +} + +/// An [`ExtensionsFactory`] that registers an [`Extension`] before a certain block. +pub struct ExtensionBeforeBlock { + before: NumberFor, + _marker: PhantomData Ext>, +} + +impl ExtensionBeforeBlock { + /// Create the extension factory. + /// + /// - `before`: The block number until the extension should be registered. + pub fn new(before: NumberFor) -> Self { + Self { before, _marker: PhantomData } + } +} + +impl ExtensionsFactory + for ExtensionBeforeBlock +{ + fn extensions_for( + &self, + _: Block::Hash, + block_number: NumberFor, + _: offchain::Capabilities, + ) -> Extensions { + let mut exts = Extensions::new(); + + if block_number < self.before { + exts.register(Ext::default()); + } + + exts + } +} + /// Create a Offchain DB accessor object. pub trait DbExternalitiesFactory: Send + Sync { /// Create [`offchain::DbExternalities`] instance. @@ -92,7 +161,7 @@ impl DbExternaliti /// This crate aggregates extensions available for the offchain calls /// and is responsible for producing a correct `Extensions` object. /// for each call, based on required `Capabilities`. -pub struct ExecutionExtensions { +pub struct ExecutionExtensions { strategies: ExecutionStrategies, keystore: Option, offchain_db: Option>, @@ -103,10 +172,10 @@ pub struct ExecutionExtensions { // That's also the reason why it's being registered lazily instead of // during initialization. transaction_pool: RwLock>>>, - extensions_factory: RwLock>, + extensions_factory: RwLock>>, } -impl Default for ExecutionExtensions { +impl Default for ExecutionExtensions { fn default() -> Self { Self { strategies: Default::default(), @@ -118,7 +187,7 @@ impl Default for ExecutionExtensions { } } -impl ExecutionExtensions { +impl ExecutionExtensions { /// Create new `ExecutionExtensions` given a `keystore` and `ExecutionStrategies`. pub fn new( strategies: ExecutionStrategies, @@ -142,8 +211,8 @@ impl ExecutionExtensions { } /// Set the new extensions_factory - pub fn set_extensions_factory(&self, maker: Box) { - *self.extensions_factory.write() = maker; + pub fn set_extensions_factory(&self, maker: impl ExtensionsFactory + 'static) { + *self.extensions_factory.write() = Box::new(maker); } /// Register transaction pool extension. @@ -156,10 +225,18 @@ impl ExecutionExtensions { /// Based on the execution context and capabilities it produces /// the extensions object to support desired set of APIs. - pub fn extensions(&self, at: &BlockId, context: ExecutionContext) -> Extensions { + pub fn extensions( + &self, + block_hash: Block::Hash, + block_number: NumberFor, + context: ExecutionContext, + ) -> Extensions { let capabilities = context.capabilities(); - let mut extensions = self.extensions_factory.read().extensions_for(capabilities); + let mut extensions = + self.extensions_factory + .read() + .extensions_for(block_hash, block_number, capabilities); if capabilities.contains(offchain::Capabilities::KEYSTORE) { if let Some(ref keystore) = self.keystore { @@ -169,10 +246,10 @@ impl ExecutionExtensions { if capabilities.contains(offchain::Capabilities::TRANSACTION_POOL) { if let Some(pool) = self.transaction_pool.read().as_ref().and_then(|x| x.upgrade()) { - extensions - .register(TransactionPoolExt( - Box::new(TransactionPoolAdapter { at: *at, pool }) as _, - )); + extensions.register(TransactionPoolExt(Box::new(TransactionPoolAdapter { + at: BlockId::Hash(block_hash), + pool, + }) as _)); } } @@ -203,7 +280,8 @@ impl ExecutionExtensions { /// the right manager and extensions object to support desired set of APIs. pub fn manager_and_extensions( &self, - at: &BlockId, + block_hash: Block::Hash, + block_number: NumberFor, context: ExecutionContext, ) -> (ExecutionManager>, Extensions) { let manager = match context { @@ -215,17 +293,17 @@ impl ExecutionExtensions { ExecutionContext::OffchainCall(_) => self.strategies.other.get_manager(), }; - (manager, self.extensions(at, context)) + (manager, self.extensions(block_hash, block_number, context)) } } /// A wrapper type to pass `BlockId` to the actual transaction pool. -struct TransactionPoolAdapter { +struct TransactionPoolAdapter { at: BlockId, pool: Arc>, } -impl offchain::TransactionPool for TransactionPoolAdapter { +impl offchain::TransactionPool for TransactionPoolAdapter { fn submit_transaction(&mut self, data: Vec) -> Result<(), ()> { let xt = match Block::Extrinsic::decode(&mut &*data) { Ok(xt) => xt, diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index a7326d57c2bf0..c1b4962d04a12 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -477,7 +477,6 @@ where "GrandpaApi_grandpa_authorities", &[], ExecutionStrategy::NativeElseWasm, - None, ) .and_then(|call_result| { Decode::decode(&mut &call_result[..]).map_err(|err| { diff --git a/client/rpc/src/state/state_full.rs b/client/rpc/src/state/state_full.rs index d6ab93f7680b0..a0bc1cfdebe06 100644 --- a/client/rpc/src/state/state_full.rs +++ b/client/rpc/src/state/state_full.rs @@ -200,7 +200,6 @@ where &method, &call_data, self.client.execution_extensions().strategies().other, - None, ) .map(Into::into) }) diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 1a16268839054..4ae1c37f6bbc9 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -310,6 +310,7 @@ where executor, spawn_handle, config.clone(), + execution_extensions, )?; crate::client::Client::new( backend, @@ -317,7 +318,6 @@ where genesis_storage, fork_blocks, bad_blocks, - execution_extensions, prometheus_registry, telemetry, config, diff --git a/client/service/src/client/call_executor.rs b/client/service/src/client/call_executor.rs index 8ab332a24be78..e9d525222b239 100644 --- a/client/service/src/client/call_executor.rs +++ b/client/service/src/client/call_executor.rs @@ -17,15 +17,15 @@ // along with this program. If not, see . use super::{client::ClientConfig, wasm_override::WasmOverride, wasm_substitutes::WasmSubstitutes}; -use sc_client_api::{backend, call_executor::CallExecutor, HeaderBackend}; +use sc_client_api::{ + backend, call_executor::CallExecutor, execution_extensions::ExecutionExtensions, HeaderBackend, +}; use sc_executor::{RuntimeVersion, RuntimeVersionOf}; -use sp_api::{ProofRecorder, StorageTransactionCache}; +use sp_api::{ExecutionContext, ProofRecorder, StorageTransactionCache}; use sp_core::traits::{CodeExecutor, RuntimeCode, SpawnNamed}; -use sp_externalities::Extensions; use sp_runtime::{generic::BlockId, traits::Block as BlockT}; use sp_state_machine::{ - backend::AsTrieBackend, ExecutionManager, ExecutionStrategy, Ext, OverlayedChanges, - StateMachine, StorageProof, + backend::AsTrieBackend, ExecutionStrategy, Ext, OverlayedChanges, StateMachine, StorageProof, }; use std::{cell::RefCell, sync::Arc}; @@ -38,6 +38,7 @@ pub struct LocalCallExecutor { wasm_substitutes: WasmSubstitutes, spawn_handle: Box, client_config: ClientConfig, + execution_extensions: Arc>, } impl LocalCallExecutor @@ -51,6 +52,7 @@ where executor: E, spawn_handle: Box, client_config: ClientConfig, + execution_extensions: ExecutionExtensions, ) -> sp_blockchain::Result { let wasm_override = client_config .wasm_runtime_overrides @@ -71,6 +73,7 @@ where spawn_handle, client_config, wasm_substitutes, + execution_extensions: Arc::new(execution_extensions), }) } @@ -124,6 +127,7 @@ where spawn_handle: self.spawn_handle.clone(), client_config: self.client_config.clone(), wasm_substitutes: self.wasm_substitutes.clone(), + execution_extensions: self.execution_extensions.clone(), } } } @@ -138,30 +142,41 @@ where type Backend = B; + fn execution_extensions(&self) -> &ExecutionExtensions { + &self.execution_extensions + } + fn call( &self, at: &BlockId, method: &str, call_data: &[u8], strategy: ExecutionStrategy, - extensions: Option, ) -> sp_blockchain::Result> { let mut changes = OverlayedChanges::default(); let at_hash = self.backend.blockchain().expect_block_hash_from_id(at)?; + let at_number = self.backend.blockchain().expect_block_number_from_id(at)?; let state = self.backend.state_at(&at_hash)?; + let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state); let runtime_code = state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; let runtime_code = self.check_override(runtime_code, at)?; + let extensions = self.execution_extensions.extensions( + at_hash, + at_number, + ExecutionContext::OffchainCall(None), + ); + let mut sm = StateMachine::new( &state, &mut changes, &self.executor, method, call_data, - extensions.unwrap_or_default(), + extensions, &runtime_code, self.spawn_handle.clone(), ) @@ -171,30 +186,25 @@ where .map_err(Into::into) } - fn contextual_call< - EM: Fn( - Result, Self::Error>, - Result, Self::Error>, - ) -> Result, Self::Error>, - >( + fn contextual_call( &self, at: &BlockId, method: &str, call_data: &[u8], changes: &RefCell, storage_transaction_cache: Option<&RefCell>>, - execution_manager: ExecutionManager, recorder: &Option>, - extensions: Option, - ) -> Result, sp_blockchain::Error> - where - ExecutionManager: Clone, - { + context: ExecutionContext, + ) -> Result, sp_blockchain::Error> { let mut storage_transaction_cache = storage_transaction_cache.map(|c| c.borrow_mut()); let at_hash = self.backend.blockchain().expect_block_hash_from_id(at)?; + let at_number = self.backend.blockchain().expect_block_number_from_id(at)?; let state = self.backend.state_at(&at_hash)?; + let (execution_manager, extensions) = + self.execution_extensions.manager_and_extensions(at_hash, at_number, context); + let changes = &mut *changes.borrow_mut(); // It is important to extract the runtime code here before we create the proof @@ -220,7 +230,7 @@ where &self.executor, method, call_data, - extensions.unwrap_or_default(), + extensions, &runtime_code, self.spawn_handle.clone(), ) @@ -235,7 +245,7 @@ where &self.executor, method, call_data, - extensions.unwrap_or_default(), + extensions, &runtime_code, self.spawn_handle.clone(), ) @@ -269,7 +279,8 @@ where call_data: &[u8], ) -> sp_blockchain::Result<(Vec, StorageProof)> { let at_hash = self.backend.blockchain().expect_block_hash_from_id(at)?; - let state = self.backend.state_at(&at_hash)?; + let at_number = self.backend.blockchain().expect_block_number_from_id(at)?; + let state = self.backend.state_at(&&at_hash)?; let trie_backend = state.as_trie_backend(); @@ -286,6 +297,11 @@ where method, call_data, &runtime_code, + self.execution_extensions.extensions( + at_hash, + at_number, + ExecutionContext::OffchainCall(None), + ), ) .map_err(Into::into) } @@ -392,6 +408,11 @@ mod tests { backend.clone(), ) .unwrap(), + execution_extensions: Arc::new(ExecutionExtensions::new( + Default::default(), + None, + None, + )), }; let check = call_executor diff --git a/client/service/src/client/client.rs b/client/service/src/client/client.rs index a7851af446b95..fd0746b2af219 100644 --- a/client/service/src/client/client.rs +++ b/client/service/src/client/client.rs @@ -67,7 +67,8 @@ use sp_keystore::SyncCryptoStorePtr; use sp_runtime::{ generic::{BlockId, SignedBlock}, traits::{ - Block as BlockT, HashFor, Header as HeaderT, NumberFor, One, SaturatedConversion, Zero, + Block as BlockT, BlockIdTo, HashFor, Header as HeaderT, NumberFor, One, + SaturatedConversion, Zero, }, BuildStorage, Digest, Justification, Justifications, StateVersion, }; @@ -113,7 +114,6 @@ where // Holds the block hash currently being imported. TODO: replace this with block queue. importing_block: RwLock>, block_rules: BlockRules, - execution_extensions: ExecutionExtensions, config: ClientConfig, telemetry: Option, _phantom: PhantomData, @@ -230,20 +230,26 @@ where Block: BlockT, B: backend::LocalBackend + 'static, { - let call_executor = - LocalCallExecutor::new(backend.clone(), executor, spawn_handle, config.clone())?; let extensions = ExecutionExtensions::new( Default::default(), keystore, sc_offchain::OffchainDb::factory_from_backend(&*backend), ); + + let call_executor = LocalCallExecutor::new( + backend.clone(), + executor, + spawn_handle, + config.clone(), + extensions, + )?; + Client::new( backend, call_executor, build_genesis_storage, Default::default(), Default::default(), - extensions, prometheus_registry, telemetry, config, @@ -347,7 +353,6 @@ where build_genesis_storage: &dyn BuildStorage, fork_blocks: ForkBlocks, bad_blocks: BadBlocks, - execution_extensions: ExecutionExtensions, prometheus_registry: Option, telemetry: Option, config: ClientConfig, @@ -394,7 +399,6 @@ where finality_actions: Default::default(), importing_block: Default::default(), block_rules: BlockRules::new(fork_blocks, bad_blocks), - execution_extensions, config, telemetry, _phantom: Default::default(), @@ -1391,7 +1395,7 @@ where } fn execution_extensions(&self) -> &ExecutionExtensions { - &self.execution_extensions + self.executor.execution_extensions() } } @@ -1585,7 +1589,7 @@ where } } -impl sp_runtime::traits::BlockIdTo for Client +impl BlockIdTo for Client where B: backend::Backend, E: CallExecutor + Send + Sync, @@ -1642,7 +1646,7 @@ where B: backend::Backend, E: CallExecutor + Send + Sync, Block: BlockT, - RA: ConstructRuntimeApi, + RA: ConstructRuntimeApi + Send + Sync, { type Api = >::RuntimeApi; @@ -1656,6 +1660,7 @@ where B: backend::Backend, E: CallExecutor + Send + Sync, Block: BlockT, + RA: Send + Sync, { type StateBackend = B::State; @@ -1663,21 +1668,15 @@ where &self, params: CallApiAtParams, ) -> Result, sp_api::ApiError> { - let at = params.at; - - let (manager, extensions) = - self.execution_extensions.manager_and_extensions(at, params.context); - self.executor .contextual_call( - at, + params.at, params.function, ¶ms.arguments, params.overlayed_changes, Some(params.storage_transaction_cache), - manager, params.recorder, - Some(extensions), + params.context, ) .map_err(Into::into) } diff --git a/client/service/test/src/client/mod.rs b/client/service/test/src/client/mod.rs index 4a5b56bd14006..6466f33046471 100644 --- a/client/service/test/src/client/mod.rs +++ b/client/service/test/src/client/mod.rs @@ -20,7 +20,8 @@ use futures::executor::block_on; use parity_scale_codec::{Decode, Encode, Joiner}; use sc_block_builder::BlockBuilderProvider; use sc_client_api::{ - in_mem, BlockBackend, BlockchainEvents, FinalityNotifications, HeaderBackend, StorageProvider, + in_mem, BlockBackend, BlockchainEvents, ExecutorProvider, FinalityNotifications, HeaderBackend, + StorageProvider, }; use sc_client_db::{Backend, BlocksPruning, DatabaseSettings, DatabaseSource, PruningMode}; use sc_consensus::{ @@ -1864,3 +1865,40 @@ fn reorg_triggers_a_notification_even_for_sources_that_should_not_trigger_notifi let tree_route = notification.tree_route.unwrap(); assert_eq!(tree_route.enacted()[0].hash, b1.hash()); } + +#[test] +fn use_dalek_ext_works() { + fn zero_ed_pub() -> sp_core::ed25519::Public { + sp_core::ed25519::Public([0u8; 32]) + } + + fn zero_ed_sig() -> sp_core::ed25519::Signature { + sp_core::ed25519::Signature::from_raw([0u8; 64]) + } + + let mut client = TestClientBuilder::new().build(); + + client.execution_extensions().set_extensions_factory( + sc_client_api::execution_extensions::ExtensionBeforeBlock::::new( + 1, + ), + ); + + let a1 = client + .new_block_at(&BlockId::Number(0), Default::default(), false) + .unwrap() + .build() + .unwrap() + .block; + block_on(client.import(BlockOrigin::NetworkInitialSync, a1.clone())).unwrap(); + + // On block zero it will use dalek and then on block 1 it will use zebra + assert!(!client + .runtime_api() + .verify_ed25519(&BlockId::Number(0), zero_ed_sig(), zero_ed_pub(), vec![]) + .unwrap()); + assert!(client + .runtime_api() + .verify_ed25519(&BlockId::Number(1), zero_ed_sig(), zero_ed_pub(), vec![]) + .unwrap()); +} diff --git a/client/tracing/src/lib.rs b/client/tracing/src/lib.rs index 1ae695a725f3f..acbde8b75da25 100644 --- a/client/tracing/src/lib.rs +++ b/client/tracing/src/lib.rs @@ -625,11 +625,7 @@ mod tests { let _guard2 = span2.enter(); // emit event tracing::event!(target: "test_target", tracing::Level::INFO, "test_event1"); - for msg in rx.recv() { - if !msg { - break - } - } + let _ = rx.recv(); // guard2 and span2 dropped / exited }); diff --git a/primitives/application-crypto/src/traits.rs b/primitives/application-crypto/src/traits.rs index 7a99c144b69f9..853208bc20cbf 100644 --- a/primitives/application-crypto/src/traits.rs +++ b/primitives/application-crypto/src/traits.rs @@ -157,8 +157,8 @@ pub trait RuntimeAppPublic: Sized { fn to_raw_vec(&self) -> Vec; } -/// Something that bound to a fixed `RuntimeAppPublic`. +/// Something that bound to a fixed [`RuntimeAppPublic`]. pub trait BoundToRuntimeAppPublic { - /// The `RuntimeAppPublic` this type is bound to. + /// The [`RuntimeAppPublic`] this type is bound to. type Public: RuntimeAppPublic; } diff --git a/primitives/externalities/src/extensions.rs b/primitives/externalities/src/extensions.rs index 5db40f12c21aa..ecb489e5ec829 100644 --- a/primitives/externalities/src/extensions.rs +++ b/primitives/externalities/src/extensions.rs @@ -89,6 +89,19 @@ macro_rules! decl_extension { Self(inner) } } + }; + ( + $( #[ $attr:meta ] )* + $vis:vis struct $ext_name:ident; + ) => { + $( #[ $attr ] )* + $vis struct $ext_name; + + impl $crate::Extension for $ext_name { + fn as_mut_any(&mut self) -> &mut dyn std::any::Any { + self + } + } } } @@ -112,7 +125,7 @@ pub trait ExtensionStore { extension: Box, ) -> Result<(), Error>; - /// Deregister extension with speicifed 'type_id' and drop it. + /// Deregister extension with specified 'type_id' and drop it. /// /// It should return error if extension is not registered. fn deregister_extension_by_type_id(&mut self, type_id: TypeId) -> Result<(), Error>; @@ -179,6 +192,13 @@ impl Extensions { } } +impl Extend for Extensions { + fn extend>(&mut self, iter: T) { + iter.into_iter() + .for_each(|ext| self.extensions.extend(ext.extensions.into_iter())); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/primitives/io/Cargo.toml b/primitives/io/Cargo.toml index 26dec17e032dd..9b30e30d7c9dd 100644 --- a/primitives/io/Cargo.toml +++ b/primitives/io/Cargo.toml @@ -34,6 +34,7 @@ parking_lot = { version = "0.12.1", optional = true } secp256k1 = { version = "0.24.0", features = ["recovery", "global-context"], optional = true } tracing = { version = "0.1.29", default-features = false } tracing-core = { version = "0.1.28", default-features = false} +ed25519-dalek = { version = "1.0.1", default-features = false, optional = true } [features] default = ["std"] @@ -57,6 +58,7 @@ std = [ "log", "futures", "parking_lot", + "ed25519-dalek", ] with-tracing = [ diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 7942bafcc2a1b..03ca7b61dd304 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -698,6 +698,34 @@ pub trait Misc { } } +#[cfg(feature = "std")] +sp_externalities::decl_extension! { + /// Extension to signal to [`crypt::ed25519_verify`] to use the dalek crate. + /// + /// The switch from `ed25519-dalek` to `ed25519-zebra` was a breaking change. + /// `ed25519-zebra` is more permissive when it comes to the verification of signatures. + /// This means that some chains may fail to sync from genesis when using `ed25519-zebra`. + /// So, this extension can be registered to the runtime execution environment to signal + /// that `ed25519-dalek` should be used for verification. The extension can be registered + /// in the following way: + /// + /// ```nocompile + /// client.execution_extensions().set_extensions_factory( + /// // Let the `UseDalekExt` extension being registered for each runtime invocation + /// // until the execution happens in the context of block `1000`. + /// sc_client_api::execution_extensions::ExtensionBeforeBlock::::new(1000) + /// ); + /// ``` + pub struct UseDalekExt; +} + +#[cfg(feature = "std")] +impl Default for UseDalekExt { + fn default() -> Self { + Self + } +} + /// Interfaces for working with crypto related types from within the runtime. #[runtime_interface] pub trait Crypto { @@ -747,13 +775,32 @@ pub trait Crypto { /// /// Returns `true` when the verification was successful. fn ed25519_verify(sig: &ed25519::Signature, msg: &[u8], pub_key: &ed25519::Public) -> bool { - ed25519::Pair::verify(sig, msg, pub_key) + // We don't want to force everyone needing to call the function in an externalities context. + // So, we assume that we should not use dalek when we are not in externalities context. + // Otherwise, we check if the extension is present. + if sp_externalities::with_externalities(|mut e| e.extension::().is_some()) + .unwrap_or_default() + { + use ed25519_dalek::Verifier; + + let public_key = if let Ok(vk) = ed25519_dalek::PublicKey::from_bytes(&pub_key.0) { + vk + } else { + return false + }; + + let sig = ed25519_dalek::Signature::from(sig.0); + + public_key.verify(msg, &sig).is_ok() + } else { + ed25519::Pair::verify(sig, msg, pub_key) + } } /// Register a `ed25519` signature for batch verification. /// /// Batch verification must be enabled by calling [`start_batch_verify`]. - /// If batch verification is not enabled, the signature will be verified immediatley. + /// If batch verification is not enabled, the signature will be verified immediately. /// To get the result of the batch verification, [`finish_batch_verify`] /// needs to be called. /// @@ -780,7 +827,7 @@ pub trait Crypto { /// Register a `sr25519` signature for batch verification. /// /// Batch verification must be enabled by calling [`start_batch_verify`]. - /// If batch verification is not enabled, the signature will be verified immediatley. + /// If batch verification is not enabled, the signature will be verified immediately. /// To get the result of the batch verification, [`finish_batch_verify`] /// needs to be called. /// @@ -2010,4 +2057,20 @@ mod tests { assert!(!crypto::finish_batch_verify()); }); } + + #[test] + fn use_dalek_ext_works() { + let mut ext = BasicExternalities::default(); + ext.register_extension(UseDalekExt::default()); + + // With dalek the zero signature should fail to verify. + ext.execute_with(|| { + assert!(!crypto::ed25519_verify(&zero_ed_sig(), &Vec::new(), &zero_ed_pub())); + }); + + // But with zebra it should work. + BasicExternalities::default().execute_with(|| { + assert!(crypto::ed25519_verify(&zero_ed_sig(), &Vec::new(), &zero_ed_pub())); + }) + } } diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 4126aea478d5b..cd3e6bad88de2 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -532,6 +532,7 @@ mod execution { method, call_data, runtime_code, + Default::default(), ) } @@ -552,6 +553,7 @@ mod execution { method: &str, call_data: &[u8], runtime_code: &RuntimeCode, + extensions: Extensions, ) -> Result<(Vec, StorageProof), Box> where S: trie_backend_essence::TrieBackendStorage, @@ -569,7 +571,7 @@ mod execution { exec, method, call_data, - Extensions::default(), + extensions, runtime_code, spawn_handle, ) diff --git a/test-utils/client/src/lib.rs b/test-utils/client/src/lib.rs index d3e71f0ad28d6..8ee652abe2c70 100644 --- a/test-utils/client/src/lib.rs +++ b/test-utils/client/src/lib.rs @@ -229,11 +229,6 @@ impl &storage, self.fork_blocks, self.bad_blocks, - ExecutionExtensions::new( - self.execution_strategies, - self.keystore, - sc_offchain::OffchainDb::factory_from_backend(&*self.backend), - ), None, None, ClientConfig { @@ -285,6 +280,11 @@ impl executor, Box::new(sp_core::testing::TaskExecutor::new()), Default::default(), + ExecutionExtensions::new( + self.execution_strategies.clone(), + self.keystore.clone(), + sc_offchain::OffchainDb::factory_from_backend(&*self.backend), + ), ) .expect("Creates LocalCallExecutor"); diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 8bda4ea602428..054b195fc6efb 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -378,6 +378,8 @@ cfg_if! { fn test_multiple_arguments(data: Vec, other: Vec, num: u32); /// Traces log "Hey I'm runtime." fn do_trace_log(); + /// Verify the given signature, public & message bundle. + fn verify_ed25519(sig: ed25519::Signature, public: ed25519::Public, message: Vec) -> bool; } } } else { @@ -428,6 +430,8 @@ cfg_if! { fn test_multiple_arguments(data: Vec, other: Vec, num: u32); /// Traces log "Hey I'm runtime." fn do_trace_log(); + /// Verify the given signature, public & message bundle. + fn verify_ed25519(sig: ed25519::Signature, public: ed25519::Public, message: Vec) -> bool; } } } @@ -863,6 +867,10 @@ cfg_if! { fn do_trace_log() { log::trace!("Hey I'm runtime"); } + + fn verify_ed25519(sig: ed25519::Signature, public: ed25519::Public, message: Vec) -> bool { + sp_io::crypto::ed25519_verify(&sig, &message, &public) + } } impl sp_consensus_aura::AuraApi for Runtime { @@ -1137,6 +1145,10 @@ cfg_if! { fn do_trace_log() { log::trace!("Hey I'm runtime: {}", log::STATIC_MAX_LEVEL); } + + fn verify_ed25519(sig: ed25519::Signature, public: ed25519::Public, message: Vec) -> bool { + sp_io::crypto::ed25519_verify(&sig, &message, &public) + } } impl sp_consensus_aura::AuraApi for Runtime {