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>

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

substrate/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);

substrate/bin/node/runtime/src/lib.rs

@@ -46,11 +46,10 @@ use sp_version::NativeVersion;
 use sp_core::OpaqueMetadata;
 use pallet_grandpa::AuthorityList as GrandpaAuthorityList;
 use pallet_grandpa::fg_primitives;
-use pallet_im_online::sr25519::{AuthorityId as ImOnlineId};
+use pallet_im_online::sr25519::AuthorityId as ImOnlineId;
 use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId;
 use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo;
 use pallet_contracts_rpc_runtime_api::ContractExecResult;
-use frame_system::offchain::TransactionSubmitter;
 use sp_inherents::{InherentData, CheckInherentsResult};
 
 #[cfg(any(feature = "std", test))]
@@ -60,6 +59,7 @@ pub use pallet_balances::Call as BalancesCall;
 pub use pallet_contracts::Gas;
 pub use frame_support::StorageValue;
 pub use pallet_staking::StakerStatus;
+use codec::Encode;
 
 /// Implementations of some helper traits passed into runtime modules as associated types.
 pub mod impls;
@@ -73,50 +73,6 @@ use constants::{time::*, currency::*};
 #[cfg(feature = "std")]
 include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs"));
 
-/// A transaction submitter with the given key type.
-pub type TransactionSubmitterOf<KeyType> = TransactionSubmitter<KeyType, Runtime, UncheckedExtrinsic>;
-
-/// Submits transaction with the node's public and signature type. Adheres to the signed extension
-/// format of the chain.
-impl frame_system::offchain::CreateTransaction<Runtime, UncheckedExtrinsic> for Runtime {
-	type Public = <Signature as traits::Verify>::Signer;
-	type Signature = Signature;
-
-	fn create_transaction<TSigner: frame_system::offchain::Signer<Self::Public, Self::Signature>>(
-		call: Call,
-		public: Self::Public,
-		account: AccountId,
-		index: Index,
-	) -> Option<(Call, <UncheckedExtrinsic as traits::Extrinsic>::SignaturePayload)> {
-		// take the biggest period possible.
-		let period = BlockHashCount::get()
-			.checked_next_power_of_two()
-			.map(|c| c / 2)
-			.unwrap_or(2) as u64;
-		let current_block = System::block_number()
-			.saturated_into::<u64>()
-			// The `System::block_number` is initialized with `n+1`,
-			// so the actual block number is `n`.
-			.saturating_sub(1);
-		let tip = 0;
-		let extra: SignedExtra = (
-			frame_system::CheckVersion::<Runtime>::new(),
-			frame_system::CheckGenesis::<Runtime>::new(),
-			frame_system::CheckEra::<Runtime>::from(generic::Era::mortal(period, current_block)),
-			frame_system::CheckNonce::<Runtime>::from(index),
-			frame_system::CheckWeight::<Runtime>::new(),
-			pallet_transaction_payment::ChargeTransactionPayment::<Runtime>::from(tip),
-			Default::default(),
-		);
-		let raw_payload = SignedPayload::new(call, extra).map_err(|e| {
-			debug::warn!("Unable to create signed payload: {:?}", e);
-		}).ok()?;
-		let signature = TSigner::sign(public, &raw_payload)?;
-		let address = Indices::unlookup(account);
-		let (call, extra, _) = raw_payload.deconstruct();
-		Some((call, (address, signature, extra)))
-	}
-}
 
 /// Runtime version.
 pub const VERSION: RuntimeVersion = RuntimeVersion {
@@ -127,7 +83,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to 0. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 243,
+	spec_version: 244,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,
@@ -362,7 +318,6 @@ impl pallet_staking::Trait for Runtime {
 	type NextNewSession = Session;
 	type ElectionLookahead = ElectionLookahead;
 	type Call = Call;
-	type SubmitTransaction = TransactionSubmitterOf<()>;
 	type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator;
 	type UnsignedPriority = StakingUnsignedPriority;
 }
@@ -549,11 +504,63 @@ parameter_types! {
 	pub const StakingUnsignedPriority: TransactionPriority = TransactionPriority::max_value() / 2;
 }
 
+
+impl<LocalCall> frame_system::offchain::CreateSignedTransaction<LocalCall> for Runtime where
+	Call: From<LocalCall>,
+{
+	fn create_transaction<C: frame_system::offchain::AppCrypto<Self::Public, Self::Signature>>(
+		call: Call,
+		public: <Signature as traits::Verify>::Signer,
+		account: AccountId,
+		nonce: Index,
+	) -> Option<(Call, <UncheckedExtrinsic as traits::Extrinsic>::SignaturePayload)> {
+		// take the biggest period possible.
+		let period = BlockHashCount::get()
+			.checked_next_power_of_two()
+			.map(|c| c / 2)
+			.unwrap_or(2) as u64;
+		let current_block = System::block_number()
+			.saturated_into::<u64>()
+			// The `System::block_number` is initialized with `n+1`,
+			// so the actual block number is `n`.
+			.saturating_sub(1);
+		let tip = 0;
+		let extra: SignedExtra = (
+			frame_system::CheckVersion::<Runtime>::new(),
+			frame_system::CheckGenesis::<Runtime>::new(),
+			frame_system::CheckEra::<Runtime>::from(generic::Era::mortal(period, current_block)),
+			frame_system::CheckNonce::<Runtime>::from(nonce),
+			frame_system::CheckWeight::<Runtime>::new(),
+			pallet_transaction_payment::ChargeTransactionPayment::<Runtime>::from(tip),
+			Default::default(),
+		);
+		let raw_payload = SignedPayload::new(call, extra).map_err(|e| {
+			debug::warn!("Unable to create signed payload: {:?}", e);
+		}).ok()?;
+		let signature = raw_payload.using_encoded(|payload| {
+			C::sign(payload, public)
+		})?;
+		let address = Indices::unlookup(account);
+		let (call, extra, _) = raw_payload.deconstruct();
+		Some((call, (address, signature.into(), extra)))
+	}
+}
+
+impl frame_system::offchain::SigningTypes for Runtime {
+	type Public = <Signature as traits::Verify>::Signer;
+	type Signature = Signature;
+}
+
+impl<C> frame_system::offchain::SendTransactionTypes<C> for Runtime where
+	Call: From<C>,
+{
+	type OverarchingCall = Call;
+	type Extrinsic = UncheckedExtrinsic;
+}
+
 impl pallet_im_online::Trait for Runtime {
 	type AuthorityId = ImOnlineId;
 	type Event = Event;
-	type Call = Call;
-	type SubmitTransaction = TransactionSubmitterOf<Self::AuthorityId>;
 	type SessionDuration = SessionDuration;
 	type ReportUnresponsiveness = Offences;
 	type UnsignedPriority = ImOnlineUnsignedPriority;
@@ -924,28 +931,14 @@ impl_runtime_apis! {
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use frame_system::offchain::{SignAndSubmitTransaction, SubmitSignedTransaction};
+	use frame_system::offchain::CreateSignedTransaction;
 
 	#[test]
 	fn validate_transaction_submitter_bounds() {
 		fn is_submit_signed_transaction<T>() where
-			T: SubmitSignedTransaction<
-				Runtime,
-				Call,
-			>,
+			T: CreateSignedTransaction<Call>,
 		{}
 
-		fn is_sign_and_submit_transaction<T>() where
-			T: SignAndSubmitTransaction<
-				Runtime,
-				Call,
-				Extrinsic=UncheckedExtrinsic,
-				CreateTransaction=Runtime,
-				Signer=ImOnlineId,
-			>,
-		{}
-
-		is_submit_signed_transaction::<TransactionSubmitterOf<ImOnlineId>>();
-		is_sign_and_submit_transaction::<TransactionSubmitterOf<ImOnlineId>>();
+		is_submit_signed_transaction::<Runtime>();
 	}
 }