Don't pass validators' public keys with attestations (#186)

* Don't pass validators' public keys with attestations

* Update statement-table's Misbehaviour typedef

* Update network/router

* Expand MessageValidationData

* Try to fix tests

* Extend ApiContext

* Remove 'index_mapping' from the SessionParams

* Construct index_mapping from authorities

* Move index_mapping to TableContext

* Add test for index_mapping order

polkadot/network/src/gossip.rs

@@ -22,7 +22,7 @@ use substrate_network::consensus_gossip::{
 	ValidatorContext,
 };
 use polkadot_validation::SignedStatement;
-use polkadot_primitives::{Block, Hash, SessionKey};
+use polkadot_primitives::{Block, Hash, SessionKey, parachain::ValidatorIndex};
 use codec::Decode;
 
 use std::collections::HashMap;
@@ -120,19 +120,26 @@ impl RegisteredMessageValidator {
 	}
 }
 
-// data needed for validating gossip.
+/// The data needed for validating gossip.
 pub(crate) struct MessageValidationData {
 	/// The authorities at a block.
 	pub(crate) authorities: Vec<SessionKey>,
+	/// Mapping from validator index to `SessionKey`.
+	pub(crate) index_mapping: HashMap<ValidatorIndex, SessionKey>,
 }
 
 impl MessageValidationData {
 	fn check_statement(&self, relay_parent: &Hash, statement: &SignedStatement) -> bool {
-		self.authorities.contains(&statement.sender) &&
+		let sender = match self.index_mapping.get(&statement.sender) {
+			Some(val) => val,
+			None => return false,
+		};
+
+		self.authorities.contains(&sender) &&
 			::polkadot_validation::check_statement(
 				&statement.statement,
 				&statement.signature,
-				statement.sender.clone(),
+				sender.clone(),
 				relay_parent,
 			)
 	}

polkadot/network/src/router.rs

@@ -27,10 +27,9 @@ use sr_primitives::traits::{ProvideRuntimeApi, BlakeTwo256, Hash as HashT};
 use polkadot_validation::{
 	SharedTable, TableRouter, SignedStatement, GenericStatement, ParachainWork, Outgoing, Validated
 };
-use polkadot_primitives::{Block, Hash, SessionKey};
-use polkadot_primitives::parachain::{
-	Extrinsic, CandidateReceipt, ParachainHost, Id as ParaId, Message,
-	Collation, PoVBlock,
+use polkadot_primitives::{Block, Hash};
+use polkadot_primitives::parachain::{Extrinsic, CandidateReceipt, ParachainHost, Id as ParaId, Message,
+	ValidatorIndex, Collation, PoVBlock,
 };
 use gossip::RegisteredMessageValidator;
 
@@ -163,12 +162,14 @@ impl<P: ProvideRuntimeApi + Send + Sync + 'static, E, N, T> Router<P, E, N, T> w
 		);
 		// dispatch future work as necessary.
 		for (producer, statement) in producers.into_iter().zip(statements) {
-			self.fetcher.knowledge().lock().note_statement(statement.sender, &statement.statement);
+			if let Some(sender) = self.table.index_to_id(statement.sender) {
+				self.fetcher.knowledge().lock().note_statement(sender, &statement.statement);
 
-			if let Some(work) = producer.map(|p| self.create_work(c_hash, p)) {
-				trace!(target: "validation", "driving statement work to completion");
-				let work = work.select2(self.fetcher.exit().clone()).then(|_| Ok(()));
-				self.fetcher.executor().spawn(work);
+				if let Some(work) = producer.map(|p| self.create_work(c_hash, p)) {
+					trace!(target: "validation", "driving statement work to completion");
+					let work = work.select2(self.fetcher.exit().clone()).then(|_| Ok(()));
+					self.fetcher.executor().spawn(work);
+				}
 			}
 		}
 	}
@@ -270,8 +271,8 @@ impl<P, E, N: NetworkService, T> Drop for Router<P, E, N, T> {
 // A unique trace for valid statements issued by a validator.
 #[derive(Hash, PartialEq, Eq, Clone, Debug)]
 enum StatementTrace {
-	Valid(SessionKey, Hash),
-	Invalid(SessionKey, Hash),
+	Valid(ValidatorIndex, Hash),
+	Invalid(ValidatorIndex, Hash),
 }
 
 // helper for deferring statements whose associated candidate is unknown.
@@ -325,19 +326,17 @@ impl DeferredStatements {
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use substrate_primitives::crypto::UncheckedInto;
-	use polkadot_primitives::parachain::ValidatorId;
 
 	#[test]
 	fn deferred_statements_works() {
 		let mut deferred = DeferredStatements::new();
 		let hash = [1; 32].into();
 		let sig = Default::default();
-		let sender: ValidatorId = [255; 32].unchecked_into();
+		let sender_index = 0;
 
 		let statement = SignedStatement {
 			statement: GenericStatement::Valid(hash),
-			sender: sender.clone(),
+			sender: sender_index,
 			signature: sig,
 		};
 
@@ -358,7 +357,7 @@ mod tests {
 
 			assert_eq!(traces.len(), 1);
 			assert_eq!(signed[0].clone(), statement);
-			assert_eq!(traces[0].clone(), StatementTrace::Valid(sender, hash));
+			assert_eq!(traces[0].clone(), StatementTrace::Valid(sender_index, hash));
 		}
 
 		// after draining

polkadot/network/src/tests/validation.rs

@@ -401,6 +401,7 @@ fn make_table(data: &ApiData, local_key: &AuthorityKeyring, parent_hash: Hash) -
 	).unwrap();
 
 	Arc::new(SharedTable::new(
+		data.validators.as_slice(),
 		group_info,
 		Arc::new(local_key.pair()),
 		parent_hash,

polkadot/network/src/validation.rs

@@ -24,10 +24,7 @@ use substrate_network::Context as NetContext;
 use substrate_network::consensus_gossip::{TopicNotification, MessageRecipient as GossipMessageRecipient};
 use polkadot_validation::{Network as ParachainNetwork, SharedTable, Collators, Statement, GenericStatement};
 use polkadot_primitives::{Block, BlockId, Hash, SessionKey};
-use polkadot_primitives::parachain::{
-	Id as ParaId, Collation, Extrinsic, ParachainHost, Message, CandidateReceipt,
-	CollatorId, ValidatorId, PoVBlock,
-};
+use polkadot_primitives::parachain::{Id as ParaId, Collation, Extrinsic, ParachainHost, Message, CandidateReceipt, CollatorId, ValidatorId, PoVBlock, ValidatorIndex};
 use codec::{Encode, Decode};
 
 use futures::prelude::*;
@@ -195,13 +192,21 @@ impl<P, E, N, T> ValidationNetwork<P, E, N, T> where
 		let task_executor = self.executor.clone();
 		let exit = self.exit.clone();
 		let message_validator = self.message_validator.clone();
+		let index_mapping = params.authorities
+			.iter()
+			.enumerate()
+			.map(|(i, k)| (i as ValidatorIndex, k.clone()))
+			.collect();
 
 		let (tx, rx) = oneshot::channel();
 		self.network.with_spec(move |spec, ctx| {
 			// before requesting messages, note live consensus session.
 			message_validator.note_session(
 				parent_hash,
-				MessageValidationData { authorities: params.authorities.clone() },
+				MessageValidationData {
+					authorities: params.authorities.clone(),
+					index_mapping,
+				},
 			);
 
 			let session = spec.new_validation_session(ctx, params);

polkadot/primitives/src/parachain.rs

@@ -38,6 +38,9 @@ pub type CollatorSignature = ed25519::Signature;
 /// so we define it to be the same type as `SessionKey`. In the future it may have different crypto.
 pub type ValidatorId = super::SessionKey;
 
+/// Index of the validator is used as a lightweight replacement of the `ValidatorId` when appropriate.
+pub type ValidatorIndex = u32;
+
  /// Signature with which parachain validators sign blocks.
 ///
 /// For now we assert that parachain validator set is exactly equivalent to the (Aura) authority set, and
@@ -279,7 +282,7 @@ pub struct AttestedCandidate {
 	/// The candidate data.
 	pub candidate: CandidateReceipt,
 	/// Validity attestations.
-	pub validity_votes: Vec<(ValidatorId, ValidityAttestation)>,
+	pub validity_votes: Vec<(ValidatorIndex, ValidityAttestation)>,
 }
 
 impl AttestedCandidate {

polkadot/runtime/src/parachains.rs

@@ -393,9 +393,10 @@ impl<T: Trait> Module<T> {
 
 			// track which voters have voted already, 1 bit per authority.
 			let mut track_voters = bitvec![0; authorities.len()];
-			for (auth_id, validity_attestation) in &candidate.validity_votes {
+			for (auth_index, validity_attestation) in &candidate.validity_votes {
+				let auth_index = *auth_index as usize;
 				// protect against double-votes.
-				match validator_group.iter().find(|&(idx, _)| &authorities[*idx] == auth_id) {
+				match validator_group.iter().find(|&(idx, _)| *idx == auth_index) {
 					None => return Err("Attesting validator not on this chain's validation duty."),
 					Some(&(idx, _)) => {
 						if track_voters.get(idx) {
@@ -427,7 +428,7 @@ impl<T: Trait> Module<T> {
 				};
 
 				ensure!(
-					sig.verify(&payload[..], &auth_id),
+					sig.verify(&payload[..], &authorities[auth_index]),
 					"Candidate validity attestation signature is bad."
 				);
 			}
@@ -479,7 +480,7 @@ mod tests {
 	use substrate_trie::NodeCodec;
 	use sr_primitives::{generic, BuildStorage};
 	use sr_primitives::traits::{BlakeTwo256, IdentityLookup};
-	use primitives::{parachain::{CandidateReceipt, HeadData, ValidityAttestation}, SessionKey};
+	use primitives::{parachain::{CandidateReceipt, HeadData, ValidityAttestation, ValidatorIndex}, SessionKey};
 	use keyring::{AuthorityKeyring, AccountKeyring};
 	use {consensus, timestamp};
 
@@ -590,7 +591,7 @@ mod tests {
 			let payload = localized_payload(statement, parent_hash);
 			let signature = key.sign(&payload[..]).into();
 
-			candidate.validity_votes.push((authorities[idx].clone(), if vote_implicit {
+			candidate.validity_votes.push((idx as ValidatorIndex, if vote_implicit {
 				ValidityAttestation::Implicit(signature)
 			} else {
 				ValidityAttestation::Explicit(signature)

polkadot/statement-table/src/generic.rs

@@ -51,7 +51,7 @@ pub trait Context {
 	/// Members are meant to submit candidates and vote on validity.
 	fn is_member_of(&self, authority: &Self::AuthorityId, group: &Self::GroupId) -> bool;
 
-	// requisite number of votes for validity from a group.
+	/// requisite number of votes for validity from a group.
 	fn requisite_votes(&self, group: &Self::GroupId) -> usize;
 }
 

polkadot/statement-table/src/lib.rs

@@ -26,7 +26,7 @@ pub mod generic;
 pub use generic::Table;
 
 use primitives::parachain::{
-	Id, CandidateReceipt, Statement as PrimitiveStatement, ValidatorSignature, ValidatorId
+	Id, CandidateReceipt, Statement as PrimitiveStatement, ValidatorSignature, ValidatorIndex,
 };
 use primitives::Hash;
 
@@ -34,10 +34,10 @@ use primitives::Hash;
 pub type Statement = generic::Statement<CandidateReceipt, Hash>;
 
 /// Signed statements about candidates.
-pub type SignedStatement = generic::SignedStatement<CandidateReceipt, Hash, ValidatorId, ValidatorSignature>;
+pub type SignedStatement = generic::SignedStatement<CandidateReceipt, Hash, ValidatorIndex, ValidatorSignature>;
 
 /// Kinds of misbehavior, along with proof.
-pub type Misbehavior = generic::Misbehavior<CandidateReceipt, Hash, ValidatorId, ValidatorSignature>;
+pub type Misbehavior = generic::Misbehavior<CandidateReceipt, Hash, ValidatorIndex, ValidatorSignature>;
 
 /// A summary of import of a statement.
 pub type Summary = generic::Summary<Hash, Id>;
@@ -46,14 +46,14 @@ pub type Summary = generic::Summary<Hash, Id>;
 pub trait Context {
 	/// Whether a authority is a member of a group.
 	/// Members are meant to submit candidates and vote on validity.
-	fn is_member_of(&self, authority: &ValidatorId, group: &Id) -> bool;
+	fn is_member_of(&self, authority: ValidatorIndex, group: &Id) -> bool;
 
-	// requisite number of votes for validity from a group.
+	/// requisite number of votes for validity from a group.
 	fn requisite_votes(&self, group: &Id) -> usize;
 }
 
 impl<C: Context> generic::Context for C {
-	type AuthorityId = ValidatorId;
+	type AuthorityId = ValidatorIndex;
 	type Digest = Hash;
 	type GroupId = Id;
 	type Signature = ValidatorSignature;
@@ -67,8 +67,8 @@ impl<C: Context> generic::Context for C {
 		candidate.parachain_index.clone()
 	}
 
-	fn is_member_of(&self, authority: &ValidatorId, group: &Id) -> bool {
-		Context::is_member_of(self, authority, group)
+	fn is_member_of(&self, authority: &Self::AuthorityId, group: &Id) -> bool {
+		Context::is_member_of(self, *authority, group)
 	}
 
 	fn requisite_votes(&self, group: &Id) -> usize {

polkadot/validation/src/lib.rs

@@ -187,9 +187,9 @@ pub trait Network {
 #[derive(Debug, Clone, Default)]
 pub struct GroupInfo {
 	/// Authorities meant to check validity of candidates.
-	pub validity_guarantors: HashSet<SessionKey>,
+	validity_guarantors: HashSet<SessionKey>,
 	/// Number of votes needed for validity.
-	pub needed_validity: usize,
+	needed_validity: usize,
 }
 
 /// Sign a table statement against a parent hash.
@@ -347,7 +347,7 @@ impl<C, N, P> ParachainValidation<C, N, P> where
 
 		debug!(target: "validation", "Active parachains: {:?}", active_parachains);
 
-		let table = Arc::new(SharedTable::new(group_info, sign_with.clone(), parent_hash, self.extrinsic_store.clone()));
+		let table = Arc::new(SharedTable::new(authorities, group_info, sign_with.clone(), parent_hash, self.extrinsic_store.clone()));
 		let router = self.network.communication_for(
 			table.clone(),
 			outgoing,

polkadot/validation/src/shared_table/mod.rs

@@ -23,9 +23,8 @@ use std::sync::Arc;
 use extrinsic_store::{Data, Store as ExtrinsicStore};
 use table::{self, Table, Context as TableContextTrait};
 use polkadot_primitives::{Block, BlockId, Hash, SessionKey};
-use polkadot_primitives::parachain::{
-	Id as ParaId, Collation, Extrinsic, CandidateReceipt,
-	AttestedCandidate, ParachainHost, PoVBlock
+use polkadot_primitives::parachain::{Id as ParaId, Collation, Extrinsic, CandidateReceipt,
+	AttestedCandidate, ParachainHost, PoVBlock, ValidatorIndex,
 };
 
 use parking_lot::Mutex;
@@ -46,11 +45,17 @@ struct TableContext {
 	parent_hash: Hash,
 	key: Arc<ed25519::Pair>,
 	groups: HashMap<ParaId, GroupInfo>,
+	index_mapping: HashMap<ValidatorIndex, SessionKey>,
 }
 
 impl table::Context for TableContext {
-	fn is_member_of(&self, authority: &SessionKey, group: &ParaId) -> bool {
-		self.groups.get(group).map_or(false, |g| g.validity_guarantors.contains(authority))
+	fn is_member_of(&self, authority: ValidatorIndex, group: &ParaId) -> bool {
+		let key = match self.index_mapping.get(&authority) {
+			Some(val) => val,
+			None => return false,
+		};
+
+		self.groups.get(group).map_or(false, |g| g.validity_guarantors.get(&key).is_some())
 	}
 
 	fn requisite_votes(&self, group: &ParaId) -> usize {
@@ -63,13 +68,23 @@ impl TableContext {
 		self.key.public().into()
 	}
 
+	fn local_index(&self) -> ValidatorIndex {
+		let id = self.local_id();
+		self
+			.index_mapping
+			.iter()
+			.find(|(_, k)| k == &&id)
+			.map(|(i, _)| *i)
+			.unwrap()
+	}
+
 	fn sign_statement(&self, statement: table::Statement) -> table::SignedStatement {
 		let signature = ::sign_table_statement(&statement, &self.key, &self.parent_hash).into();
 
 		table::SignedStatement {
 			statement,
 			signature,
-			sender: self.local_id(),
+			sender: self.local_index(),
 		}
 	}
 }
@@ -131,9 +146,9 @@ impl SharedTableInner {
 
 		self.update_trackers(&summary.candidate, context);
 
-		let local_id = context.local_id();
+		let local_index = context.local_index();
 
-		let para_member = context.is_member_of(&local_id, &summary.group_id);
+		let para_member = context.is_member_of(local_index, &summary.group_id);
 
 		let digest = &summary.candidate;
 
@@ -375,13 +390,15 @@ impl SharedTable {
 	/// Provide the key to sign with, and the parent hash of the relay chain
 	/// block being built.
 	pub fn new(
+		authorities: &[ed25519::Public],
 		groups: HashMap<ParaId, GroupInfo>,
 		key: Arc<ed25519::Pair>,
 		parent_hash: Hash,
 		extrinsic_store: ExtrinsicStore,
 	) -> Self {
-		SharedTable {
-			context: Arc::new(TableContext { groups, key, parent_hash }),
+		let index_mapping = authorities.iter().enumerate().map(|(i, k)| (i as ValidatorIndex, k.clone())).collect();
+		Self {
+			context: Arc::new(TableContext { groups, key, parent_hash, index_mapping, }),
 			inner: Arc::new(Mutex::new(SharedTableInner {
 				table: Table::default(),
 				validated: HashMap::new(),
@@ -513,7 +530,7 @@ impl SharedTable {
 	}
 
 	/// Get all witnessed misbehavior.
-	pub fn get_misbehavior(&self) -> HashMap<SessionKey, table::Misbehavior> {
+	pub fn get_misbehavior(&self) -> HashMap<ValidatorIndex, table::Misbehavior> {
 		self.inner.lock().table.get_misbehavior().clone()
 	}
 
@@ -534,6 +551,11 @@ impl SharedTable {
 
 		rx
 	}
+
+	/// Returns id of the validator corresponding to the given index.
+	pub fn index_to_id(&self, index: ValidatorIndex) -> Option<SessionKey> {
+		self.context.index_mapping.get(&index).cloned()
+	}
 }
 
 #[cfg(test)]
@@ -577,13 +599,15 @@ mod tests {
 
 		let validity_other_key = AuthorityKeyring::Bob.pair();
 		let validity_other = validity_other_key.public();
+		let validity_other_index = 1;
 
 		groups.insert(para_id, GroupInfo {
-			validity_guarantors: [local_id, validity_other.clone()].iter().cloned().collect(),
+			validity_guarantors: [local_id.clone(), validity_other.clone()].iter().cloned().collect(),
 			needed_validity: 2,
 		});
 
 		let shared_table = SharedTable::new(
+			&[local_id, validity_other],
 			groups,
 			local_key.clone(),
 			parent_hash,
@@ -607,7 +631,7 @@ mod tests {
 		let signed_statement = ::table::generic::SignedStatement {
 			statement: candidate_statement,
 			signature: signature.into(),
-			sender: validity_other,
+			sender: validity_other_index,
 		};
 
 		shared_table.import_remote_statement(
@@ -628,13 +652,15 @@ mod tests {
 
 		let validity_other_key = AuthorityKeyring::Bob.pair();
 		let validity_other = validity_other_key.public();
+		let validity_other_index = 1;
 
 		groups.insert(para_id, GroupInfo {
-			validity_guarantors: [local_id, validity_other.clone()].iter().cloned().collect(),
+			validity_guarantors: [local_id.clone(), validity_other.clone()].iter().cloned().collect(),
 			needed_validity: 1,
 		});
 
 		let shared_table = SharedTable::new(
+			&[local_id, validity_other],
 			groups,
 			local_key.clone(),
 			parent_hash,
@@ -658,7 +684,7 @@ mod tests {
 		let signed_statement = ::table::generic::SignedStatement {
 			statement: candidate_statement,
 			signature: signature.into(),
-			sender: validity_other,
+			sender: validity_other_index,
 		};
 
 		shared_table.import_remote_statement(
@@ -758,13 +784,15 @@ mod tests {
 
 		let validity_other_key = AuthorityKeyring::Bob.pair();
 		let validity_other = validity_other_key.public();
+		let validity_other_index = 1;
 
 		groups.insert(para_id, GroupInfo {
-			validity_guarantors: [local_id, validity_other.clone()].iter().cloned().collect(),
+			validity_guarantors: [local_id.clone(), validity_other.clone()].iter().cloned().collect(),
 			needed_validity: 1,
 		});
 
 		let shared_table = SharedTable::new(
+			&[local_id, validity_other],
 			groups,
 			local_key.clone(),
 			parent_hash,
@@ -789,7 +817,7 @@ mod tests {
 		let signed_statement = ::table::generic::SignedStatement {
 			statement: candidate_statement,
 			signature: signature.into(),
-			sender: validity_other,
+			sender: validity_other_index,
 		};
 
 		let _a = shared_table.import_remote_statement(
@@ -823,11 +851,12 @@ mod tests {
 		let validity_other = validity_other_key.public();
 
 		groups.insert(para_id, GroupInfo {
-			validity_guarantors: [local_id, validity_other].iter().cloned().collect(),
+			validity_guarantors: [local_id.clone(), validity_other.clone()].iter().cloned().collect(),
 			needed_validity: 1,
 		});
 
 		let shared_table = SharedTable::new(
+			&[local_id, validity_other],
 			groups,
 			local_key.clone(),
 			parent_hash,
@@ -861,4 +890,28 @@ mod tests {
 
 		assert!(a.is_none());
 	}
+
+	#[test]
+	fn index_mapping_from_authorities() {
+		let authorities_set: &[&[_]] = &[
+			&[],
+			&[AuthorityKeyring::Alice.pair().public()],
+			&[AuthorityKeyring::Alice.pair().public(), AuthorityKeyring::Bob.pair().public()],
+			&[AuthorityKeyring::Bob.pair().public(), AuthorityKeyring::Alice.pair().public()],
+			&[AuthorityKeyring::Alice.pair().public(), AuthorityKeyring::Bob.pair().public(), AuthorityKeyring::Charlie.pair().public()],
+			&[AuthorityKeyring::Charlie.pair().public(), AuthorityKeyring::Bob.pair().public(), AuthorityKeyring::Alice.pair().public()],
+		];
+
+		for authorities in authorities_set {
+			let shared_table = SharedTable::new(
+				authorities,
+				HashMap::new(),
+				Arc::new(AuthorityKeyring::Alice.pair()),
+				Default::default(),
+				ExtrinsicStore::new_in_memory(),
+			);
+			let expected_mapping = authorities.iter().enumerate().map(|(i, k)| (i as ValidatorIndex, k.clone())).collect();
+			assert_eq!(shared_table.context.index_mapping, expected_mapping);
+		}
+	}
 }