ed25519_verify: Support using dalek for historical blocks (#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 <123550+andresilva@users.noreply.github.com>

* Fix merge and warning

* Fix docs

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

substrate/primitives/application-crypto/src/traits.rs

@@ -157,8 +157,8 @@ pub trait RuntimeAppPublic: Sized {
 	fn to_raw_vec(&self) -> Vec<u8>;
 }
 
-/// 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;
 }

substrate/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<dyn Extension>,
 	) -> 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<Extensions> for Extensions {
+	fn extend<T: IntoIterator<Item = Extensions>>(&mut self, iter: T) {
+		iter.into_iter()
+			.for_each(|ext| self.extensions.extend(ext.extensions.into_iter()));
+	}
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;

substrate/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 = [

substrate/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::<Block, UseDalekExt>::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::<UseDalekExt>().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.
 	///
@@ -1977,4 +2024,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()));
+		})
+	}
 }

substrate/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<u8>, StorageProof), Box<dyn Error>>
 	where
 		S: trie_backend_essence::TrieBackendStorage<H>,
@@ -569,7 +571,7 @@ mod execution {
 			exec,
 			method,
 			call_data,
-			Extensions::default(),
+			extensions,
 			runtime_code,
 			spawn_handle,
 		)