Add assignment keys to session keys, no separate approvals key

node/test/service/src/chain_spec.rs

@@ -20,7 +20,7 @@ use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId;
 use babe_primitives::AuthorityId as BabeId;
 use grandpa::AuthorityId as GrandpaId;
 use pallet_staking::Forcing;
-use polkadot_primitives::v1::{ValidatorId, AccountId};
+use polkadot_primitives::v1::{ValidatorId, AccountId, AssignmentId};
 use polkadot_service::chain_spec::{get_account_id_from_seed, get_from_seed, Extensions};
 use polkadot_test_runtime::constants::currency::DOTS;
 use sc_chain_spec::{ChainSpec, ChainType};
@@ -63,13 +63,14 @@ pub fn polkadot_local_testnet_genesis() -> polkadot_test_runtime::GenesisConfig
 /// Helper function to generate stash, controller and session key from seed
 fn get_authority_keys_from_seed(
 	seed: &str,
-) -> (AccountId, AccountId, BabeId, GrandpaId, ValidatorId, AuthorityDiscoveryId) {
+) -> (AccountId, AccountId, BabeId, GrandpaId, ValidatorId, AssignmentId, AuthorityDiscoveryId) {
 	(
 		get_account_id_from_seed::<sr25519::Public>(&format!("{}//stash", seed)),
 		get_account_id_from_seed::<sr25519::Public>(seed),
 		get_from_seed::<BabeId>(seed),
 		get_from_seed::<GrandpaId>(seed),
 		get_from_seed::<ValidatorId>(seed),
+		get_from_seed::<AssignmentId>(seed),
 		get_from_seed::<AuthorityDiscoveryId>(seed),
 	)
 }
@@ -93,7 +94,15 @@ fn testnet_accounts() -> Vec<AccountId> {
 
 /// Helper function to create polkadot GenesisConfig for testing
 fn polkadot_testnet_genesis(
-	initial_authorities: Vec<(AccountId, AccountId, BabeId, GrandpaId, ValidatorId, AuthorityDiscoveryId)>,
+	initial_authorities: Vec<(
+		AccountId,
+		AccountId,
+		BabeId,
+		GrandpaId,
+		ValidatorId,
+		AssignmentId,
+		AuthorityDiscoveryId,
+	)>,
 	root_key: AccountId,
 	endowed_accounts: Option<Vec<AccountId>>,
 ) -> polkadot_test_runtime::GenesisConfig {
@@ -126,8 +135,9 @@ fn polkadot_testnet_genesis(
 						runtime::SessionKeys {
 							babe: x.2.clone(),
 							grandpa: x.3.clone(),
-							parachain_validator: x.4.clone(),
-							authority_discovery: x.5.clone(),
+							para_validator: x.4.clone(),
+							para_assignment: x.5.clone(),
+							authority_discovery: x.6.clone(),
 						},
 					)
 				})

primitives/src/v1.rs

@@ -58,19 +58,6 @@ pub use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId;
 /// Unique identifier for the Inclusion Inherent
 pub const INCLUSION_INHERENT_IDENTIFIER: InherentIdentifier = *b"inclusn0";
 
-
-/// The key type ID for a parachain approval voting key.
-pub const APPROVAL_KEY_TYPE_ID: KeyTypeId = KeyTypeId(*b"aprv");
-
-mod approval_app {
-	use application_crypto::{app_crypto, sr25519};
-	app_crypto!(sr25519, super::APPROVAL_KEY_TYPE_ID);
-}
-
-/// The public key of a keypair used by a validator for approval voting
-/// on included parachain candidates.
-pub type ApprovalId = approval_app::Public;
-
 /// The key type ID for parachain assignment key.
 pub const ASSIGNMENT_KEY_TYPE_ID: KeyTypeId = KeyTypeId(*b"asgn");
 
@@ -85,7 +72,6 @@ mod assigment_app {
 /// to approve included parachain candidates.
 pub type AssignmentId = assigment_app::Public;
 
-
 /// Get a collator signature payload on a relay-parent, block-data combo.
 pub fn collator_signature_payload<H: AsRef<[u8]>>(
 	relay_parent: &H,
@@ -698,8 +684,8 @@ pub struct SessionInfo {
 	pub validators: Vec<ValidatorId>,
 	/// Validators' authority discovery keys for the session in canonical ordering.
 	pub discovery_keys: Vec<AuthorityDiscoveryId>,
-	/// The assignment and approval keys for validators.
-	pub approval_keys: Vec<(ApprovalId, AssignmentId)>,
+	/// The assignment keys for validators.
+	pub assignment_keys: Vec<AssignmentId>,
 	/// Validators in shuffled ordering - these are the validator groups as produced
 	/// by the `Scheduler` module for the session and are typically referred to by
 	/// `GroupIndex`.

roadmap/implementers-guide/src/node/approval/approval-voting.md

@@ -281,5 +281,5 @@ enum RequiredTranches {
   * Fetch the block entry and candidate entry. Ignore if `None` - we've probably just lost a race with finality.
   * Construct a `SignedApprovalVote` with the validator index for the session.
   * `import_checked_approval(block_entry, candidate_entry, validator_index)`
-  * Construct a `IndirectSignedApprovalVote` using the informatio about the vote.
+  * Construct a `IndirectSignedApprovalVote` using the information about the vote.
   * Dispatch `ApprovalDistributionMessage::DistributeApproval`.

roadmap/implementers-guide/src/protocol-approval.md

@@ -38,7 +38,7 @@ We need two separate keys for the approval subsystem:
 
 - **Approval assignment keys** are sr25519/schnorrkel keys used only for the assignment criteria VRFs.  We implicitly sign assignment notices with approval assignment keys by including their relay chain context and additional data in the VRF's extra message, but exclude these from its VRF input.  
 
-- **Approval vote keys** would only sign off on candidate parablock validity and has no natural key type restrictions.  We could reuse the ed25519 grandpa keys for this purpose since these signatures control access to grandpa, although distant future node configurations might favor separate roles.
+- **Approval vote keys** would only sign off on candidate parablock validity and has no natural key type restrictions. There's no need for this to actualy embody a new session key type. We just want to make a distinction between assignments and approvals, although distant future node configurations might favor separate roles. We re-use the same keys as are used for parachain backing in practice.
 
 Approval vote keys could relatively easily be handled by some hardened signer tooling, perhaps even HSMs assuming we select ed25519 for approval vote keys.  Approval assignment keys might or might not support hardened signer tooling, but doing so sounds far more complex.  In fact, assignment keys determine only VRF outputs that determine approval checker assignments, for which they can only act or not act, so they cannot equivocate, lie, etc. and represent little if any slashing risk for validator operators. 
 

roadmap/implementers-guide/src/runtime/session_info.md

@@ -8,12 +8,13 @@ Helper structs:
 
 ```rust
 struct SessionInfo {
-    // validators in canonical ordering.
+    // validators in canonical ordering. These are the public keys used for backing,
+    // dispute participation, and approvals.
     validators: Vec<ValidatorId>,
     // validators' authority discovery keys for the session in canonical ordering.
     discovery_keys: Vec<DiscoveryId>,
-    // The assignment and approval keys for validators.
-    approval_keys: Vec<(AssignmentId, ApprovalId)>,
+    // The assignment keys for validators.
+    assignment_keys: Vec<AssignmentId>,
     // validators in shuffled ordering - these are the validator groups as produced
     // by the `Scheduler` module for the session and are typically referred to by
     // `GroupIndex`.

roadmap/implementers-guide/src/types/approval.md

@@ -1,9 +1,5 @@
 # Approval Types
 
-## ApprovalId
-
-The public key of a keypair used by a validator for approval voting on included parachain candidates.
-
 ## AssignmentId
 
 The public key of a keypair used by a validator for determining assignments to approve included parachain candidates.
@@ -57,11 +53,13 @@ struct ApprovalVote(Hash);
 
 ## SignedApprovalVote
 
+An approval vote signed with a validator's key. This should be verifiable under the `ValidatorId` corresponding to the `ValidatorIndex` of the session, which should be implicit from context.
+
 ```rust
 struct SignedApprovalVote {
     vote: ApprovalVote,
     validator: ValidatorIndex,
-    signature: ApprovalSignature,
+    signature: ValidatorSignature,
 }
 ```
 
@@ -78,7 +76,7 @@ struct IndirectSignedApprovalVote {
     // The index of the candidate in the list of candidates fully included as-of the block.
     candidate_index: u32,
     validator: ValidatorIndex,
-    signature: ApprovalSignature,
+    signature: ValidatorSignature,
 }
 ```
 

runtime/common/src/lib.rs

@@ -27,7 +27,7 @@ pub mod impls;
 pub mod paras_sudo_wrapper;
 pub mod paras_registrar;
 
-use primitives::v1::{BlockNumber, ValidatorId};
+use primitives::v1::{BlockNumber, ValidatorId, AssignmentId};
 use sp_runtime::{Perquintill, Perbill, FixedPointNumber};
 use frame_system::limits;
 use frame_support::{
@@ -158,6 +158,35 @@ impl<T: pallet_session::Config>
 	fn on_disabled(_: usize) { }
 }
 
+/// A placeholder since there is currently no provided session key handler for parachain validator
+/// keys.
+pub struct AssignmentSessionKeyPlaceholder<T>(sp_std::marker::PhantomData<T>);
+impl<T> sp_runtime::BoundToRuntimeAppPublic for AssignmentSessionKeyPlaceholder<T> {
+	type Public = AssignmentId;
+}
+
+impl<T: pallet_session::Config>
+	pallet_session::OneSessionHandler<T::AccountId> for AssignmentSessionKeyPlaceholder<T>
+{
+	type Key = AssignmentId;
+
+	fn on_genesis_session<'a, I: 'a>(_validators: I) where
+		I: Iterator<Item = (&'a T::AccountId, AssignmentId)>,
+		T::AccountId: 'a
+	{
+
+	}
+
+	fn on_new_session<'a, I: 'a>(_changed: bool, _v: I, _q: I) where
+		I: Iterator<Item = (&'a T::AccountId, AssignmentId)>,
+		T::AccountId: 'a
+	{
+
+	}
+
+	fn on_disabled(_: usize) { }
+}
+
 #[cfg(test)]
 mod multiplier_tests {
 	use super::*;

runtime/kusama/src/lib.rs

@@ -29,13 +29,13 @@ use primitives::v1::{
 	AccountId, AccountIndex, Balance, BlockNumber, CandidateEvent, CommittedCandidateReceipt,
 	CoreState, GroupRotationInfo, Hash, Id, Moment, Nonce, OccupiedCoreAssumption,
 	PersistedValidationData, Signature, ValidationCode, ValidationData, ValidatorId, ValidatorIndex,
-	InboundDownwardMessage, InboundHrmpMessage, SessionInfo,
+	InboundDownwardMessage, InboundHrmpMessage, SessionInfo, AssignmentId,
 };
 use runtime_common::{
 	claims, SlowAdjustingFeeUpdate, CurrencyToVote,
 	impls::DealWithFees,
 	BlockHashCount, RocksDbWeight, BlockWeights, BlockLength, OffchainSolutionWeightLimit,
-	ParachainSessionKeyPlaceholder,
+	ParachainSessionKeyPlaceholder, AssignmentSessionKeyPlaceholder,
 };
 use sp_runtime::{
 	create_runtime_str, generic, impl_opaque_keys, ModuleId,
@@ -90,7 +90,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("kusama"),
 	impl_name: create_runtime_str!("parity-kusama"),
 	authoring_version: 2,
-	spec_version: 2027,
+	spec_version: 2028,
 	impl_version: 0,
 	#[cfg(not(feature = "disable-runtime-api"))]
 	apis: RUNTIME_API_VERSIONS,
@@ -262,16 +262,46 @@ parameter_types! {
 	pub const Offset: BlockNumber = 0;
 }
 
+impl_opaque_keys! {
+	pub struct OldSessionKeys {
+		pub grandpa: Grandpa,
+		pub babe: Babe,
+		pub im_online: ImOnline,
+		pub para_validator: ParachainSessionKeyPlaceholder<Runtime>,
+		pub authority_discovery: AuthorityDiscovery,
+	}
+}
+
 impl_opaque_keys! {
 	pub struct SessionKeys {
 		pub grandpa: Grandpa,
 		pub babe: Babe,
 		pub im_online: ImOnline,
-		pub parachain_validator: ParachainSessionKeyPlaceholder<Runtime>,
+		pub para_validator: ParachainSessionKeyPlaceholder<Runtime>,
+		pub para_assignment: AssignmentSessionKeyPlaceholder<Runtime>,
 		pub authority_discovery: AuthorityDiscovery,
 	}
 }
 
+fn transform_session_keys(v: AccountId, old: OldSessionKeys) -> SessionKeys {
+	SessionKeys {
+		grandpa: old.grandpa,
+		babe: old.babe,
+		im_online: old.im_online,
+		para_validator: old.para_validator,
+		para_assignment: {
+			// We need to produce a dummy value that's unique for the validator.
+			let mut id = AssignmentId::default();
+			let id_raw: &mut [u8] = id.as_mut();
+			id_raw.copy_from_slice(v.as_ref());
+			id_raw[0..4].copy_from_slice(b"asgn");
+
+			id
+		},
+		authority_discovery: old.authority_discovery,
+	}
+}
+
 parameter_types! {
 	pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(17);
 }
@@ -1105,6 +1135,15 @@ impl frame_support::traits::OnRuntimeUpgrade for FixCouncilHistoricalVotes {
 	}
 }
 
+// When this is removed, should also remove `OldSessionKeys`.
+pub struct UpgradeSessionKeys;
+impl frame_support::traits::OnRuntimeUpgrade for UpgradeSessionKeys {
+	fn on_runtime_upgrade() -> frame_support::weights::Weight {
+		Session::upgrade_keys::<OldSessionKeys, _>(transform_session_keys);
+		Perbill::from_percent(50) * BlockWeights::get().max_block
+	}
+}
+
 pub struct CustomOnRuntimeUpgrade;
 impl frame_support::traits::OnRuntimeUpgrade for CustomOnRuntimeUpgrade {
 	fn on_runtime_upgrade() -> frame_support::weights::Weight {
@@ -1208,7 +1247,7 @@ pub type Executive = frame_executive::Executive<
 	frame_system::ChainContext<Runtime>,
 	Runtime,
 	AllModules,
-	FixCouncilHistoricalVotes,
+	UpgradeSessionKeys,
 >;
 /// The payload being signed in the transactions.
 pub type SignedPayload = generic::SignedPayload<Call, SignedExtra>;

runtime/parachains/src/session_info.rs

@@ -87,9 +87,7 @@ impl<T: Config> Module<T> {
 
 		let validators = notification.validators.clone();
 		let discovery_keys = <T as AuthorityDiscoveryConfig>::authorities();
-		let _assignment_keys = AssignmentKeysUnsafe::get();
-		// FIXME: remove this once https://github.com/paritytech/polkadot/pull/2092 is merged
-		let approval_keys = Default::default();
+		let assignment_keys = AssignmentKeysUnsafe::get();
 		let validator_groups = <scheduler::Module<T>>::validator_groups();
 		let n_cores = n_parachains + config.parathread_cores;
 		let zeroth_delay_tranche_width = config.zeroth_delay_tranche_width;
@@ -118,7 +116,7 @@ impl<T: Config> Module<T> {
 		let new_session_info = SessionInfo {
 			validators,
 			discovery_keys,
-			approval_keys,
+			assignment_keys,
 			validator_groups,
 			n_cores,
 			zeroth_delay_tranche_width,