Refactors the offchain worker api (#3150)

* Update offchain primitives. * Update offchain worker. * Update im-online. * Update service. * Update node and node-template. * Update runtime version. * Fix build. * Fix offchain worker tests. * Generalize authority_pubkey. * Add test. * Update lib.rs
2026-06-12 21:41:12 +00:00 · 2019-07-22 11:20:57 +02:00
parent 2edeef5825
commit a3d19baea3
17 changed files with 509 additions and 327 deletions
@@ -23,11 +23,11 @@ use parity_codec::{Encode, Decode};
 use primitives::offchain::{
 	Timestamp, HttpRequestId, HttpRequestStatus, HttpError,
 	Externalities as OffchainExt,
-	CryptoKind, CryptoKeyId,
+	CryptoKind, CryptoKey,
 	StorageKind,
 	OpaqueNetworkState, OpaquePeerId, OpaqueMultiaddr,
 };
-use primitives::crypto::{Pair, Protected};
+use primitives::crypto::{Pair, Public, Protected};
 use primitives::{ed25519, sr25519};
 use runtime_primitives::{
 	generic::BlockId,
@@ -44,25 +44,144 @@ enum ExtMessage {

 /// A persisted key seed.
 #[derive(Encode, Decode)]
-struct CryptoKey {
+struct StoredKey {
 	kind: CryptoKind,
 	phrase: String,
 }

-enum Key {
-	Sr25519(sr25519::Pair),
+impl StoredKey {
+	fn generate_with_phrase(kind: CryptoKind, password: Option<&str>) -> Self {
+		match kind {
+			CryptoKind::Ed25519 => {
+				let phrase = ed25519::Pair::generate_with_phrase(password).1;
+				Self { kind, phrase }
+			}
+			CryptoKind::Sr25519 => {
+				let phrase = sr25519::Pair::generate_with_phrase(password).1;
+				Self { kind, phrase }
+			}
+		}
+	}
+
+	fn to_local_key(&self, password: Option<&str>) -> Result<LocalKey, ()> {
+		match self.kind {
+			CryptoKind::Ed25519 => {
+				ed25519::Pair::from_phrase(&self.phrase, password)
+					.map(|x| LocalKey::Ed25519(x.0))
+			}
+			CryptoKind::Sr25519 => {
+				sr25519::Pair::from_phrase(&self.phrase, password)
+					.map(|x| LocalKey::Sr25519(x.0))
+			}
+		}
+		.map_err(|e| {
+			warn!("Error recovering Offchain Worker key. Password invalid? {:?}", e);
+			()
+		})
+	}
+}
+
+enum LocalKey {
 	Ed25519(ed25519::Pair),
+	Sr25519(sr25519::Pair),
+}
+
+impl LocalKey {
+	fn public(&self) -> Result<Vec<u8>, ()> {
+		match self {
+			LocalKey::Ed25519(pair) => Ok(pair.public().to_raw_vec()),
+			LocalKey::Sr25519(pair) => Ok(pair.public().to_raw_vec()),
+		}
+	}
+
+	fn sign(&self, data: &[u8]) -> Result<Vec<u8>, ()> {
+		match self {
+			LocalKey::Ed25519(pair) => {
+				let sig = pair.sign(data);
+				let bytes: &[u8] = sig.as_ref();
+				Ok(bytes.to_vec())
+			}
+			LocalKey::Sr25519(pair) => {
+				let sig = pair.sign(data);
+				let bytes: &[u8] = sig.as_ref();
+				Ok(bytes.to_vec())
+			}
+		}
+	}
+
+	fn verify(&self, msg: &[u8], signature: &[u8]) -> Result<bool, ()> {
+		match self {
+			LocalKey::Ed25519(pair) => {
+				Ok(ed25519::Pair::verify_weak(signature, msg, pair.public()))
+			}
+			LocalKey::Sr25519(pair) => {
+				Ok(sr25519::Pair::verify_weak(signature, msg, pair.public()))
+			}
+		}
+	}
+}
+
+/// A key.
+enum Key<ConsensusPair, FinalityPair> {
+	LocalKey(LocalKey),
+	AuthorityKey(ConsensusPair),
+	FgAuthorityKey(FinalityPair),
+}
+
+impl<ConsensusPair: Pair, FinalityPair: Pair> Key<ConsensusPair, FinalityPair> {
+	fn public(&self) -> Result<Vec<u8>, ()> {
+		match self {
+			Key::LocalKey(local) => {
+				local.public()
+			}
+			Key::AuthorityKey(pair) => {
+				Ok(pair.public().to_raw_vec())
+			}
+			Key::FgAuthorityKey(pair) => {
+				Ok(pair.public().to_raw_vec())
+			}
+		}
+	}
+
+	fn sign(&self, data: &[u8]) -> Result<Vec<u8>, ()> {
+		match self {
+			Key::LocalKey(local) => {
+				local.sign(data)
+			}
+			Key::AuthorityKey(pair) => {
+				Ok(pair.sign(data).as_ref().to_vec())
+			}
+			Key::FgAuthorityKey(pair) => {
+				Ok(pair.sign(data).as_ref().to_vec())
+			}
+		}
+	}
+
+	fn verify(&self, msg: &[u8], signature: &[u8]) -> Result<bool, ()> {
+		match self {
+			Key::LocalKey(local) => {
+				local.verify(msg, signature)
+			}
+			Key::AuthorityKey(pair) => {
+				Ok(ConsensusPair::verify_weak(signature, msg, pair.public()))
+			}
+			Key::FgAuthorityKey(pair) => {
+				Ok(FinalityPair::verify_weak(signature, msg, pair.public()))
+			}
+		}
+	}
 }

 /// Asynchronous offchain API.
 ///
 /// NOTE this is done to prevent recursive calls into the runtime (which are not supported currently).
-pub(crate) struct Api<Storage, KeyProvider> {
+pub(crate) struct Api<Storage, KeyProvider, Block: traits::Block> {
 	sender: mpsc::UnboundedSender<ExtMessage>,
 	db: Storage,
 	keys_password: Protected<String>,
 	key_provider: KeyProvider,
 	network_state: Arc<dyn NetworkStateInfo + Send + Sync>,
+	at: BlockId<Block>,
 }

 fn unavailable_yet<R: Default>(name: &str) -> R {
@@ -77,55 +196,56 @@ const KEYS_PREFIX: &[u8] = b"keys";

 const NEXT_ID: &[u8] = b"crypto_key_id";

-impl<Storage, KeyProvider> Api<Storage, KeyProvider> where
+impl<Storage, KeyProvider, Block> Api<Storage, KeyProvider, Block> where
 	Storage: OffchainStorage,
-	KeyProvider: AuthorityKeyProvider,
+	KeyProvider: AuthorityKeyProvider<Block>,
+	Block: traits::Block,
 {
-	fn keypair<P: Pair>(&self, phrase: &str) -> Result<P, ()> {
-		P::from_phrase(phrase, Some(self.keys_password.as_ref()))
-			.map_err(|e| {
-				warn!("Error recovering Offchain Worker key. Password invalid? {:?}", e);
-				()
-			})
-			.map(|x| x.0)
+	fn password(&self) -> Option<&str> {
+		Some(self.keys_password.as_ref().as_str())
 	}

-	fn read_key(&self, id: Option<CryptoKeyId>, kind: CryptoKind) -> Result<Key, ()> {
-		if let Some(id) = id {
-			let key = self.db.get(KEYS_PREFIX, &id.0.encode())
-				.and_then(|key| CryptoKey::decode(&mut &*key))
-				.ok_or(())?;
-			if key.kind != kind {
-				warn!(
-					"Invalid crypto kind (got: {:?}, expected: {:?}), when requesting key {:?}",
-					key.kind,
-					kind,
-					id
-				);
-				return Err(())
+	fn read_key(
+		&self,
+		key: CryptoKey,
+	) -> Result<Key<KeyProvider::ConsensusPair, KeyProvider::FinalityPair>, ()> {
+		match key {
+			CryptoKey::LocalKey { id, kind } => {
+				let key = self.db.get(KEYS_PREFIX, &id.encode())
+					.and_then(|key| StoredKey::decode(&mut &*key))
+					.ok_or(())?;
+				if key.kind != kind {
+					warn!(
+						"Invalid crypto kind (got: {:?}, expected: {:?}), when requesting key {:?}",
+						key.kind,
+						kind,
+						id
+					);
+					return Err(())
+				}
+				Ok(Key::LocalKey(key.to_local_key(self.password())?))
+			}
+			CryptoKey::AuthorityKey => {
+				let key = self.key_provider
+					.authority_key(&self.at)
+					.ok_or(())?;
+				Ok(Key::AuthorityKey(key))
+			}
+			CryptoKey::FgAuthorityKey => {
+				let key = self.key_provider
+					.fg_authority_key(&self.at)
+					.ok_or(())?;
+				Ok(Key::FgAuthorityKey(key))
 			}
-
-			Ok(match key.kind {
-				CryptoKind::Sr25519 => Key::Sr25519(self.keypair(&key.phrase)?),
-				CryptoKind::Ed25519 => Key::Ed25519(self.keypair(&key.phrase)?),
-			})
-		} else {
-			let key = match kind {
-				CryptoKind::Sr25519 => self.key_provider.authority_key().map(Key::Sr25519),
-				CryptoKind::Ed25519 => self.key_provider.authority_key().map(Key::Ed25519),
-			};
-
-			key.ok_or_else(|| {
-				warn!("AuthorityKey is not configured, yet offchain worker tried to access it.");
-				()
-			})
 		}
 	}
 }

-impl<Storage, KeyProvider> OffchainExt for Api<Storage, KeyProvider> where
+impl<Storage, KeyProvider, Block> OffchainExt for Api<Storage, KeyProvider, Block>
+where
 	Storage: OffchainStorage,
-	KeyProvider: AuthorityKeyProvider,
+	KeyProvider: AuthorityKeyProvider<Block>,
+	Block: traits::Block,
 {
 	fn submit_transaction(&mut self, ext: Vec<u8>) -> Result<(), ()> {
 		self.sender
@@ -134,16 +254,8 @@ impl<Storage, KeyProvider> OffchainExt for Api<Storage, KeyProvider> where
 			.map_err(|_| ())
 	}

-	fn new_crypto_key(&mut self, kind: CryptoKind) -> Result<CryptoKeyId, ()> {
-		let phrase = match kind {
-			CryptoKind::Ed25519 => {
-				ed25519::Pair::generate_with_phrase(Some(self.keys_password.as_ref())).1
-			},
-			CryptoKind::Sr25519 => {
-				sr25519::Pair::generate_with_phrase(Some(self.keys_password.as_ref())).1
-			},
-		};
-
+	fn new_crypto_key(&mut self, kind: CryptoKind) -> Result<CryptoKey, ()> {
+		let key = StoredKey::generate_with_phrase(kind, self.password());
 		let (id, id_encoded) = loop {
 			let encoded = self.db.get(KEYS_PREFIX, NEXT_ID);
 			let encoded_slice = encoded.as_ref().map(|x| x.as_slice());
@@ -157,19 +269,13 @@ impl<Storage, KeyProvider> OffchainExt for Api<Storage, KeyProvider> where
 			}
 		};

-		self.db.set(KEYS_PREFIX, &id_encoded, &CryptoKey { phrase, kind } .encode());
+		self.db.set(KEYS_PREFIX, &id_encoded, &key.encode());

-		Ok(CryptoKeyId(id))
+		Ok(CryptoKey::LocalKey { id, kind })
 	}

-	fn authority_pubkey(&self, kind: CryptoKind) -> Result<Vec<u8>, ()> {
-		let key = self.read_key(None, kind)?;
-		let public = match key {
-			Key::Sr25519(pair) => pair.public().encode(),
-			Key::Ed25519(pair) => pair.public().encode(),
-		};
-
-		Ok(public)
+	fn pubkey(&self, key: CryptoKey) -> Result<Vec<u8>, ()> {
+		self.read_key(key)?.public()
 	}

 	fn network_state(&self) -> Result<OpaqueNetworkState, ()> {
@@ -182,33 +288,23 @@ impl<Storage, KeyProvider> OffchainExt for Api<Storage, KeyProvider> where
 		Ok(OpaqueNetworkState::from(state))
 	}

-	fn encrypt(&mut self, _key: Option<CryptoKeyId>, _kind: CryptoKind, _data: &[u8]) -> Result<Vec<u8>, ()> {
+	fn encrypt(&mut self, _key: CryptoKey, _data: &[u8]) -> Result<Vec<u8>, ()> {
 		unavailable_yet::<()>("encrypt");
 		Err(())
 	}

-	fn decrypt(&mut self, _key: Option<CryptoKeyId>, _kind: CryptoKind, _data: &[u8]) -> Result<Vec<u8>, ()> {
+	fn decrypt(&mut self, _key: CryptoKey, _data: &[u8]) -> Result<Vec<u8>, ()> {
 		unavailable_yet::<()>("decrypt");
 		Err(())

 	}

-	fn sign(&mut self, key: Option<CryptoKeyId>, kind: CryptoKind, data: &[u8]) -> Result<Vec<u8>, ()> {
-		let key = self.read_key(key, kind)?;
-
-		Ok(match key {
-			Key::Sr25519(pair) => pair.sign(data).0.to_vec(),
-			Key::Ed25519(pair) => pair.sign(data).0.to_vec(),
-		})
+	fn sign(&mut self, key: CryptoKey, data: &[u8]) -> Result<Vec<u8>, ()> {
+		self.read_key(key)?.sign(data)
 	}

-	fn verify(&mut self, key: Option<CryptoKeyId>, kind: CryptoKind, msg: &[u8], signature: &[u8]) -> Result<bool, ()> {
-		let key = self.read_key(key, kind)?;
-
-		Ok(match key {
-			Key::Sr25519(pair) => sr25519::Pair::verify_weak(signature, msg, pair.public()),
-			Key::Ed25519(pair) => ed25519::Pair::verify_weak(signature, msg, pair.public()),
-		})
+	fn verify(&mut self, key: CryptoKey, msg: &[u8], signature: &[u8]) -> Result<bool, ()> {
+		self.read_key(key)?.verify(msg, signature)
 	}

 	fn timestamp(&mut self) -> Timestamp {
@@ -385,14 +481,14 @@ pub(crate) struct AsyncApi<A: ChainApi> {

 impl<A: ChainApi> AsyncApi<A> {
 	/// Creates new Offchain extensions API implementation  an the asynchronous processing part.
-	pub fn new<S: OffchainStorage, P: AuthorityKeyProvider>(
+	pub fn new<S: OffchainStorage, P: AuthorityKeyProvider<A::Block>>(
 		transaction_pool: Arc<Pool<A>>,
 		db: S,
 		keys_password: Protected<String>,
 		key_provider: P,
 		at: BlockId<A::Block>,
 		network_state: Arc<dyn NetworkStateInfo + Send + Sync>,
-	) -> (Api<S, P>, AsyncApi<A>) {
+	) -> (Api<S, P, A::Block>, AsyncApi<A>) {
 		let (sender, rx) = mpsc::unbounded();

 		let api = Api {
@@ -401,6 +497,7 @@ impl<A: ChainApi> AsyncApi<A> {
 			keys_password,
 			key_provider,
 			network_state,
+			at,
 		};

 		let async_api = AsyncApi {
@@ -446,10 +543,12 @@ impl<A: ChainApi> AsyncApi<A> {
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use std::{collections::HashSet, convert::TryFrom};
+	use std::convert::TryFrom;
+	use runtime_primitives::traits::Zero;
 	use client_db::offchain::LocalStorage;
 	use crate::tests::TestProvider;
 	use network::PeerId;
+	use test_client::runtime::Block;

 	struct MockNetworkStateInfo();

@@ -463,7 +562,7 @@ mod tests {
 		}
 	}

-	fn offchain_api() -> (Api<LocalStorage, TestProvider>, AsyncApi<impl ChainApi>) {
+	fn offchain_api() -> (Api<LocalStorage, TestProvider<Block>, Block>, AsyncApi<impl ChainApi>) {
 		let _ = env_logger::try_init();
 		let db = LocalStorage::new_test();
 		let client = Arc::new(test_client::new());
@@ -472,7 +571,7 @@ mod tests {
 		);

 		let mock = Arc::new(MockNetworkStateInfo());
-		AsyncApi::new(pool, db, "pass".to_owned().into(), TestProvider::default(), BlockId::Number(0), mock)
+		AsyncApi::new(pool, db, "pass".to_owned().into(), TestProvider::default(), BlockId::Number(Zero::zero()), mock)
 	}

 	#[test]
@@ -515,22 +614,16 @@ mod tests {
 			let msg = b"Hello world!";

 			// when
-			let key_id = api.new_crypto_key(kind).unwrap();
-			let signature = api.sign(Some(key_id), kind, msg).unwrap();
+			let key = api.new_crypto_key(kind).unwrap();
+			let signature = api.sign(key, msg).unwrap();

 			// then
-			let res = api.verify(Some(key_id), kind, msg, &signature).unwrap();
+			let res = api.verify(key, msg, &signature).unwrap();
 			assert_eq!(res, true);
-			let res = api.verify(Some(key_id), kind, msg, &[]).unwrap();
+			let res = api.verify(key, msg, &[]).unwrap();
 			assert_eq!(res, false);
-			let res = api.verify(Some(key_id), kind, b"Different msg", &signature).unwrap();
+			let res = api.verify(key, b"Different msg", &signature).unwrap();
 			assert_eq!(res, false);
-
-			assert_eq!(
-				api.verify(Some(key_id), CryptoKind::Sr25519, msg, &signature).is_err(),
-				kind != CryptoKind::Sr25519
-			);
-
 		};

 		test(CryptoKind::Ed25519);
@@ -543,23 +636,17 @@ mod tests {
 		let mut api = offchain_api().0;
 		api.key_provider.ed_key = Some(ed25519::Pair::generate().0);
 		let msg = b"Hello world!";
-		let kind = CryptoKind::Ed25519;

 		// when
-		let signature = api.sign(None, kind, msg).unwrap();
+		let signature = api.sign(CryptoKey::AuthorityKey, msg).unwrap();

 		// then
-		let res = api.verify(None, kind, msg, &signature).unwrap();
+		let res = api.verify(CryptoKey::AuthorityKey, msg, &signature).unwrap();
 		assert_eq!(res, true);
-		let res = api.verify(None, kind, msg, &[]).unwrap();
+		let res = api.verify(CryptoKey::AuthorityKey, msg, &[]).unwrap();
 		assert_eq!(res, false);
-		let res = api.verify(None, kind, b"Different msg", &signature).unwrap();
+		let res = api.verify(CryptoKey::AuthorityKey, b"Different msg", &signature).unwrap();
 		assert_eq!(res, false);
-
-		assert!(
-			api.verify(None, CryptoKind::Sr25519, msg, &signature).is_err(),
-			"Invalid kind should trigger a missing key error."
-		);
 	}

 	#[test]