Offchain signing (#5182)

* New approach to offchain signing. * Use in im-online * Rewrite to use Account<T> * DRY signing. * Implement send_raw_unsigned_transaction * WiP * Expunge LocalCall * Expunge LocalCall * Fix compilation. * Solve call. * Make it compile again. * Finalize implementation. * Change CreateTransaction * Clear CreateTransaction. * Add price payload * Send raw transaction * Submit signed payload / unsigned transaction (WIP) * Supertrait requirements on T::Signature * Validate signature of payload on an unsigned transaction * Fix encoding - part 1 * Make it compile. * Fix compilation of unsigned validator. * Pass price payload to the transaction * Make block number part of the signed payload * Send signed transaction * Implement all_accounts, any_account * Fix formatting * Implement submit_transaction * Submit signed transaction (ForAll, ForAny) * Fix formatting * Implement CreateSignedTransaction * Move sign and verify to AppCrypto * Sign transaction * Call `use_encoded` * Remove SubmitAndSignTransaction * Implement runtime using new SigningTypes * Adapt offchain example to changes * Fix im-online pallet * Quick fix: rename AuthorityId2 * Fix offchain example tests * Add a comment on why keystore is required in unsigned transaction test * Use UintAuthorityId instead of u64 * WIP * Remove IdentifyAccount from UintAuthorityId * Implement PublicWrapper type * Fix im-online tests * Fix runtime test * Bump spec version * Fix executor tests * Rename ImOnlineAuthId -> ImOnlineAuthorityId and formatting * Fix merge * Documentation * Revert u64 -> UintAuthorityId conversion * Fix string errors * Document public members in offchain module * Introduce SubmitTransaction * Update pallets to use SubmitTransaction * WIP * Use SubmitTransaction in offchain * Use `submit_unsigned_transaction` * Fix tests * Update docs * Remove SigningTypes requirement from `SendTransactionTypes` * Fix tests * Update frame/system/src/offchain.rs Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/system/src/offchain.rs Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/example-offchain-worker/src/tests.rs Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/system/src/offchain.rs Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/system/src/offchain.rs Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Remove leftover from previous iterations * Change enum to struct * Remove public * Move mock to node/executor/tests * Cleanup test-helpers * Make `application-crypto` `std` feature internal The macros should not generate code that requires that the calling crate has a feature with the name `std` defined. * Revert cargo lock update * Use TestAuthorityId from common * Restore members of account to public * Tidy up imports * Fix benchmarking pallet * Add tests demonstrating ForAll, ForAny on signer * Move definition of AppCrypto in example-offchain-worker from tests to mod::crypto * Cleanup stray comment * Fix ValidTransaction * Re-fix CreateSignedTransaction * Address PR feedback * Add can_sign method to signer * Propagate error * Improve documentation * Fix vec! macro not available * Document SendTransactiontypes * Add some docs. * Split signing examples * Add tests for signing examples * WIP can_sign - PR feedback * WIP * Split for_any / for_all into different calls * Verify payload and signature in test * Fix can_sign implementation * Fix impl_version * Import Box from sp_std * Create issues for TODOs * Ignore doctest. * Add test directly to system. Adjust UintTypes. * Add some tests to account filtering. * Remove code samples and point to example offchain worker * Fix doc links * Fix im-online tests using signatures. Co-authored-by: Tomasz Drwięga <tomasz@parity.io> Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: Bastian Köcher <git@kchr.de>
paritytech · Apr 21, 2020 · b20dc82 · b20dc82
1 parent ededdc4
commit b20dc82
Show file tree

Hide file tree

Showing 18 changed files with 1,469 additions and 606 deletions.
diff --git a/bin/node/executor/tests/common.rs b/bin/node/executor/tests/common.rs
@@ -15,10 +15,21 @@
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
 use codec::{Encode, Decode};
+use frame_system::offchain::AppCrypto;
 use frame_support::Hashable;
 use sp_state_machine::TestExternalities as CoreTestExternalities;
-use sp_core::{NeverNativeValue, NativeOrEncoded, traits::{CodeExecutor, RuntimeCode}};
-use sp_runtime::{ApplyExtrinsicResult, traits::{Header as HeaderT, BlakeTwo256}};
+use sp_core::{
+	NeverNativeValue, NativeOrEncoded,
+	crypto::KeyTypeId,
+	sr25519::Signature,
+	traits::{CodeExecutor, RuntimeCode},
+};
+use sp_runtime::{
+	ApplyExtrinsicResult,
+	MultiSigner,
+	MultiSignature,
+	traits::{Header as HeaderT, BlakeTwo256},
+};
 use sc_executor::{NativeExecutor, WasmExecutionMethod};
 use sc_executor::error::Result;
 
@@ -31,6 +42,25 @@ use node_primitives::{Hash, BlockNumber};
 use node_testing::keyring::*;
 use sp_externalities::Externalities;
 
+pub const TEST_KEY_TYPE_ID: KeyTypeId = KeyTypeId(*b"test");
+
+pub mod sr25519 {
+	mod app_sr25519 {
+		use sp_application_crypto::{app_crypto, sr25519};
+		use super::super::TEST_KEY_TYPE_ID;
+		app_crypto!(sr25519, TEST_KEY_TYPE_ID);
+	}
+
+	pub type AuthorityId = app_sr25519::Public;
+}
+
+pub struct TestAuthorityId;
+impl AppCrypto<MultiSigner, MultiSignature> for TestAuthorityId {
+	type RuntimeAppPublic = sr25519::AuthorityId;
+	type GenericSignature = Signature;
+	type GenericPublic = sp_core::sr25519::Public;
+}
+
 /// The wasm runtime code.
 ///
 /// `compact` since it is after post-processing with wasm-gc which performs tree-shaking thus

diff --git a/bin/node/executor/tests/submit_transaction.rs b/bin/node/executor/tests/submit_transaction.rs
@@ -15,24 +15,29 @@
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
 use node_runtime::{
-	Call, Executive, Indices, Runtime, TransactionSubmitterOf, UncheckedExtrinsic,
+	Executive, Indices, Runtime, UncheckedExtrinsic,
 };
 use sp_application_crypto::AppKey;
 use sp_core::testing::KeyStore;
-use sp_core::traits::KeystoreExt;
-use sp_core::offchain::{
-	TransactionPoolExt,
-	testing::TestTransactionPoolExt,
+use sp_core::{
+	offchain::{
+		TransactionPoolExt,
+		testing::TestTransactionPoolExt,
+	},
+	traits::KeystoreExt,
+};
+use frame_system::{
+	offchain::{
+		Signer,
+		SubmitTransaction,
+		SendSignedTransaction,
+	}
 };
-use frame_system::offchain::{SubmitSignedTransaction, SubmitUnsignedTransaction};
-use pallet_im_online::sr25519::AuthorityPair as Key;
 use codec::Decode;
 
 pub mod common;
 use self::common::*;
 
-type SubmitTransaction = TransactionSubmitterOf<pallet_im_online::sr25519::AuthorityId>;
-
 #[test]
 fn should_submit_unsigned_transaction() {
 	let mut t = new_test_ext(COMPACT_CODE, false);
@@ -49,8 +54,7 @@ fn should_submit_unsigned_transaction() {
 		};
 
 		let call = pallet_im_online::Call::heartbeat(heartbeat_data, signature);
-		<SubmitTransaction as SubmitUnsignedTransaction<Runtime, Call>>
-			::submit_unsigned(call)
+		SubmitTransaction::<Runtime, pallet_im_online::Call<Runtime>>::submit_unsigned_transaction(call.into())
 			.unwrap();
 
 		assert_eq!(state.read().transactions.len(), 1)
@@ -66,23 +70,16 @@ fn should_submit_signed_transaction() {
 	t.register_extension(TransactionPoolExt::new(pool));
 
 	let keystore = KeyStore::new();
-	keystore.write().sr25519_generate_new(Key::ID, Some(&format!("{}/hunter1", PHRASE))).unwrap();
-	keystore.write().sr25519_generate_new(Key::ID, Some(&format!("{}/hunter2", PHRASE))).unwrap();
-	keystore.write().sr25519_generate_new(Key::ID, Some(&format!("{}/hunter3", PHRASE))).unwrap();
+	keystore.write().sr25519_generate_new(sr25519::AuthorityId::ID, Some(&format!("{}/hunter1", PHRASE))).unwrap();
+	keystore.write().sr25519_generate_new(sr25519::AuthorityId::ID, Some(&format!("{}/hunter2", PHRASE))).unwrap();
+	keystore.write().sr25519_generate_new(sr25519::AuthorityId::ID, Some(&format!("{}/hunter3", PHRASE))).unwrap();
 	t.register_extension(KeystoreExt(keystore));
 
 	t.execute_with(|| {
-		let keys = <SubmitTransaction as SubmitSignedTransaction<Runtime, Call>>
-			::find_all_local_keys();
-		assert_eq!(keys.len(), 3, "Missing keys: {:?}", keys);
-
-		let can_sign = <SubmitTransaction as SubmitSignedTransaction<Runtime, Call>>
-			::can_sign();
-		assert!(can_sign, "Since there are keys, `can_sign` should return true");
-
-		let call = pallet_balances::Call::transfer(Default::default(), Default::default());
-		let results =
-			<SubmitTransaction as SubmitSignedTransaction<Runtime, Call>>::submit_signed(call);
+		let results = Signer::<Runtime, TestAuthorityId>::all_accounts()
+			.send_signed_transaction(|_| {
+				pallet_balances::Call::transfer(Default::default(), Default::default())
+			});
 
 		let len = results.len();
 		assert_eq!(len, 3);
@@ -98,27 +95,26 @@ fn should_submit_signed_twice_from_the_same_account() {
 	t.register_extension(TransactionPoolExt::new(pool));
 
 	let keystore = KeyStore::new();
-	keystore.write().sr25519_generate_new(Key::ID, Some(&format!("{}/hunter1", PHRASE))).unwrap();
+	keystore.write().sr25519_generate_new(sr25519::AuthorityId::ID, Some(&format!("{}/hunter1", PHRASE))).unwrap();
+	keystore.write().sr25519_generate_new(sr25519::AuthorityId::ID, Some(&format!("{}/hunter2", PHRASE))).unwrap();
 	t.register_extension(KeystoreExt(keystore));
 
 	t.execute_with(|| {
-		let call = pallet_balances::Call::transfer(Default::default(), Default::default());
-		let results =
-			<SubmitTransaction as SubmitSignedTransaction<Runtime, Call>>::submit_signed(call);
+		let result = Signer::<Runtime, TestAuthorityId>::any_account()
+			.send_signed_transaction(|_| {
+				pallet_balances::Call::transfer(Default::default(), Default::default())
+			});
 
-		let len = results.len();
-		assert_eq!(len, 1);
-		assert_eq!(results.into_iter().filter_map(|x| x.1.ok()).count(), len);
+		assert!(result.is_some());
 		assert_eq!(state.read().transactions.len(), 1);
 
 		// submit another one from the same account. The nonce should be incremented.
-		let call = pallet_balances::Call::transfer(Default::default(), Default::default());
-		let results =
-			<SubmitTransaction as SubmitSignedTransaction<Runtime, Call>>::submit_signed(call);
+		let result = Signer::<Runtime, TestAuthorityId>::any_account()
+			.send_signed_transaction(|_| {
+				pallet_balances::Call::transfer(Default::default(), Default::default())
+			});
 
-		let len = results.len();
-		assert_eq!(len, 1);
-		assert_eq!(results.into_iter().filter_map(|x| x.1.ok()).count(), len);
+		assert!(result.is_some());
 		assert_eq!(state.read().transactions.len(), 2);
 
 		// now check that the transaction nonces are not equal
@@ -136,6 +132,60 @@ fn should_submit_signed_twice_from_the_same_account() {
 	});
 }
 
+#[test]
+fn should_submit_signed_twice_from_all_accounts() {
+	let mut t = new_test_ext(COMPACT_CODE, false);
+	let (pool, state) = TestTransactionPoolExt::new();
+	t.register_extension(TransactionPoolExt::new(pool));
+
+	let keystore = KeyStore::new();
+	keystore.write().sr25519_generate_new(sr25519::AuthorityId::ID, Some(&format!("{}/hunter1", PHRASE))).unwrap();
+	keystore.write().sr25519_generate_new(sr25519::AuthorityId::ID, Some(&format!("{}/hunter2", PHRASE))).unwrap();
+	t.register_extension(KeystoreExt(keystore));
+
+	t.execute_with(|| {
+		let results = Signer::<Runtime, TestAuthorityId>::all_accounts()
+			.send_signed_transaction(|_| {
+				pallet_balances::Call::transfer(Default::default(), Default::default())
+			});
+
+		let len = results.len();
+		assert_eq!(len, 2);
+		assert_eq!(results.into_iter().filter_map(|x| x.1.ok()).count(), len);
+		assert_eq!(state.read().transactions.len(), 2);
+
+		// submit another one from the same account. The nonce should be incremented.
+		let results = Signer::<Runtime, TestAuthorityId>::all_accounts()
+			.send_signed_transaction(|_| {
+				pallet_balances::Call::transfer(Default::default(), Default::default())
+			});
+
+		let len = results.len();
+		assert_eq!(len, 2);
+		assert_eq!(results.into_iter().filter_map(|x| x.1.ok()).count(), len);
+		assert_eq!(state.read().transactions.len(), 4);
+
+		// now check that the transaction nonces are not equal
+		let s = state.read();
+		fn nonce(tx: UncheckedExtrinsic) -> frame_system::CheckNonce<Runtime> {
+			let extra = tx.signature.unwrap().2;
+			extra.3
+		}
+		let nonce1 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[0]).unwrap());
+		let nonce2 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[1]).unwrap());
+		let nonce3 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[2]).unwrap());
+		let nonce4 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[3]).unwrap());
+		assert!(
+			nonce1 != nonce3,
+			"Transactions should have different nonces. Got: 1st tx nonce: {:?}, 2nd nonce: {:?}", nonce1, nonce3
+		);
+		assert!(
+			nonce2 != nonce4,
+			"Transactions should have different nonces. Got: 1st tx nonce: {:?}, 2nd tx nonce: {:?}", nonce2, nonce4
+		);
+	});
+}
+
 #[test]
 fn submitted_transaction_should_be_valid() {
 	use codec::Encode;
@@ -148,13 +198,14 @@ fn submitted_transaction_should_be_valid() {
 	t.register_extension(TransactionPoolExt::new(pool));
 
 	let keystore = KeyStore::new();
-	keystore.write().sr25519_generate_new(Key::ID, Some(&format!("{}/hunter1", PHRASE))).unwrap();
+	keystore.write().sr25519_generate_new(sr25519::AuthorityId::ID, Some(&format!("{}/hunter1", PHRASE))).unwrap();
 	t.register_extension(KeystoreExt(keystore));
 
 	t.execute_with(|| {
-		let call = pallet_balances::Call::transfer(Default::default(), Default::default());
-		let results =
-			<SubmitTransaction as SubmitSignedTransaction<Runtime, Call>>::submit_signed(call);
+		let results = Signer::<Runtime, TestAuthorityId>::all_accounts()
+			.send_signed_transaction(|_| {
+				pallet_balances::Call::transfer(Default::default(), Default::default())
+			});
 		let len = results.len();
 		assert_eq!(len, 1);
 		assert_eq!(results.into_iter().filter_map(|x| x.1.ok()).count(), len);