Commit 41f62652 authored by Stanislav Tkach's avatar Stanislav Tkach Committed by asynchronous rob
Browse files

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
parent 987da512
Pipeline #37162 passed with stages
in 11 minutes and 31 seconds
......@@ -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,
)
}
......
......@@ -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
......
......@@ -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,
......
......@@ -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);
......
......@@ -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 {
......
......@@ -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)
......
......@@ -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;
}
......
......@@ -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 {
......
......@@ -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,
......
......@@ -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);
}
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment