ed25519_verify: Support using dalek for historical blocks (paritytech#12661)

* 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::<Block, sp_io::UseDalekExt>::new(BLOCK_NUMBER_UNTIL_DALEK_SHOULD_BE_USED)
);
```

* Fix doc

* More fixes

* Update client/api/src/execution_extensions.rs

Co-authored-by: André Silva <[email protected]>

* Fix merge and warning

* Fix docs

Co-authored-by: André Silva <[email protected]>

Author: 2 people

Cargo.lock

@@ -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<Block: BlockT> {
@@ -47,6 +46,9 @@ pub trait CallExecutor<B: BlockT>: RuntimeVersionOf {
 	/// The backend used by the node.
 	type Backend: crate::backend::Backend<B>;
 
+	/// Returns the [`ExecutionExtensions`].
+	fn execution_extensions(&self) -> &ExecutionExtensions<B>;
+
 	/// Execute a call to a contract on top of state in a block of given hash.
 	///
 	/// No changes are made.
@@ -56,20 +58,14 @@ pub trait CallExecutor<B: BlockT>: RuntimeVersionOf {
 		method: &str,
 		call_data: &[u8],
 		strategy: ExecutionStrategy,
-		extensions: Option<Extensions>,
 	) -> Result<Vec<u8>, sp_blockchain::Error>;
 
 	/// Execute a contextual call on top of state in a block of a given hash.
 	///
 	/// 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<Vec<u8>, Self::Error>,
-			Result<Vec<u8>, Self::Error>,
-		) -> Result<Vec<u8>, Self::Error>,
-	>(
+	fn contextual_call(
 		&self,
 		at: &BlockId<B>,
 		method: &str,
@@ -80,12 +76,9 @@ pub trait CallExecutor<B: BlockT>: RuntimeVersionOf {
 				StorageTransactionCache<B, <Self::Backend as crate::backend::Backend<B>>::State>,
 			>,
 		>,
-		execution_manager: ExecutionManager<EM>,
 		proof_recorder: &Option<ProofRecorder<B>>,
-		extensions: Option<Extensions>,
-	) -> sp_blockchain::Result<Vec<u8>>
-	where
-		ExecutionManager<EM>: Clone;
+		context: ExecutionContext,
+	) -> sp_blockchain::Result<Vec<u8>>;
 
 	/// Extract RuntimeVersion of given block
 	///

client/api/src/call_executor.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<Block: BlockT>: 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<Block>,
+		capabilities: offchain::Capabilities,
+	) -> Extensions;
 }
 
-impl ExtensionsFactory for () {
-	fn extensions_for(&self, _capabilities: offchain::Capabilities) -> Extensions {
+impl<Block: BlockT> ExtensionsFactory<Block> for () {
+	fn extensions_for(
+		&self,
+		_: Block::Hash,
+		_: NumberFor<Block>,
+		_capabilities: offchain::Capabilities,
+	) -> Extensions {
 		Extensions::new()
 	}
 }
 
+impl<Block: BlockT, T: ExtensionsFactory<Block>> ExtensionsFactory<Block> for Vec<T> {
+	fn extensions_for(
+		&self,
+		block_hash: Block::Hash,
+		block_number: NumberFor<Block>,
+		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<Block: BlockT, Ext> {
+	before: NumberFor<Block>,
+	_marker: PhantomData<fn(Ext) -> Ext>,
+}
+
+impl<Block: BlockT, Ext> ExtensionBeforeBlock<Block, Ext> {
+	/// Create the extension factory.
+	///
+	/// - `before`: The block number until the extension should be registered.
+	pub fn new(before: NumberFor<Block>) -> Self {
+		Self { before, _marker: PhantomData }
+	}
+}
+
+impl<Block: BlockT, Ext: Default + Extension> ExtensionsFactory<Block>
+	for ExtensionBeforeBlock<Block, Ext>
+{
+	fn extensions_for(
+		&self,
+		_: Block::Hash,
+		block_number: NumberFor<Block>,
+		_: 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<T: offchain::DbExternalities + Clone + Sync + Send + 'static> 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<Block: traits::Block> {
+pub struct ExecutionExtensions<Block: BlockT> {
 	strategies: ExecutionStrategies,
 	keystore: Option<SyncCryptoStorePtr>,
 	offchain_db: Option<Box<dyn DbExternalitiesFactory>>,
@@ -103,10 +172,10 @@ pub struct ExecutionExtensions<Block: traits::Block> {
 	// That's also the reason why it's being registered lazily instead of
 	// during initialization.
 	transaction_pool: RwLock<Option<Weak<dyn OffchainSubmitTransaction<Block>>>>,
-	extensions_factory: RwLock<Box<dyn ExtensionsFactory>>,
+	extensions_factory: RwLock<Box<dyn ExtensionsFactory<Block>>>,
 }
 
-impl<Block: traits::Block> Default for ExecutionExtensions<Block> {
+impl<Block: BlockT> Default for ExecutionExtensions<Block> {
 	fn default() -> Self {
 		Self {
 			strategies: Default::default(),
@@ -118,7 +187,7 @@ impl<Block: traits::Block> Default for ExecutionExtensions<Block> {
 	}
 }
 
-impl<Block: traits::Block> ExecutionExtensions<Block> {
+impl<Block: BlockT> ExecutionExtensions<Block> {
 	/// Create new `ExecutionExtensions` given a `keystore` and `ExecutionStrategies`.
 	pub fn new(
 		strategies: ExecutionStrategies,
@@ -142,8 +211,8 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 	}
 
 	/// Set the new extensions_factory
-	pub fn set_extensions_factory(&self, maker: Box<dyn ExtensionsFactory>) {
-		*self.extensions_factory.write() = maker;
+	pub fn set_extensions_factory(&self, maker: impl ExtensionsFactory<Block> + 'static) {
+		*self.extensions_factory.write() = Box::new(maker);
 	}
 
 	/// Register transaction pool extension.
@@ -156,10 +225,18 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 
 	/// Based on the execution context and capabilities it produces
 	/// the extensions object to support desired set of APIs.
-	pub fn extensions(&self, at: &BlockId<Block>, context: ExecutionContext) -> Extensions {
+	pub fn extensions(
+		&self,
+		block_hash: Block::Hash,
+		block_number: NumberFor<Block>,
+		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<Block: traits::Block> ExecutionExtensions<Block> {
 
 		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<Block: traits::Block> ExecutionExtensions<Block> {
 	/// the right manager and extensions object to support desired set of APIs.
 	pub fn manager_and_extensions<E: std::fmt::Debug>(
 		&self,
-		at: &BlockId<Block>,
+		block_hash: Block::Hash,
+		block_number: NumberFor<Block>,
 		context: ExecutionContext,
 	) -> (ExecutionManager<DefaultHandler<E>>, Extensions) {
 		let manager = match context {
@@ -215,17 +293,17 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 			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<Block: traits::Block> {
+struct TransactionPoolAdapter<Block: BlockT> {
 	at: BlockId<Block>,
 	pool: Arc<dyn OffchainSubmitTransaction<Block>>,
 }
 
-impl<Block: traits::Block> offchain::TransactionPool for TransactionPoolAdapter<Block> {
+impl<Block: BlockT> offchain::TransactionPool for TransactionPoolAdapter<Block> {
 	fn submit_transaction(&mut self, data: Vec<u8>) -> Result<(), ()> {
 		let xt = match Block::Extrinsic::decode(&mut &*data) {
 			Ok(xt) => xt,

client/api/src/execution_extensions.rs

@@ -477,7 +477,6 @@ where
 				"GrandpaApi_grandpa_authorities",
 				&[],
 				ExecutionStrategy::NativeElseWasm,
-				None,
 			)
 			.and_then(|call_result| {
 				Decode::decode(&mut &call_result[..]).map_err(|err| {

client/finality-grandpa/src/lib.rs

@@ -200,7 +200,6 @@ where
 						&method,
 						&call_data,
 						self.client.execution_extensions().strategies().other,
-						None,
 					)
 					.map(Into::into)
 			})

client/rpc/src/state/state_full.rs

@@ -311,14 +311,14 @@ where
 		executor,
 		spawn_handle,
 		config.clone(),
+		execution_extensions,
 	)?;
 	crate::client::Client::new(
 		backend,
 		executor,
 		genesis_storage,
 		fork_blocks,
 		bad_blocks,
-		execution_extensions,
 		prometheus_registry,
 		telemetry,
 		config,