From 525ff77e4a6edce0d118dddd98b397e094081af6 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Thu, 18 Jun 2020 13:36:05 +0200 Subject: [PATCH 01/11] add signed wrapper, typedef SignedStatement --- .../implementors-guide/src/types/backing.md | 61 +++++++++++++++---- 1 file changed, 48 insertions(+), 13 deletions(-) diff --git a/roadmap/implementors-guide/src/types/backing.md b/roadmap/implementors-guide/src/types/backing.md index 46f1d79e87b1..f77cb450eed0 100644 --- a/roadmap/implementors-guide/src/types/backing.md +++ b/roadmap/implementors-guide/src/types/backing.md @@ -19,6 +19,33 @@ enum ValidityAttestation { } ``` +## Signed Wrapper + +There are a few distinct types which we desire to sign, and validate the signatures of. Instead of duplicating this work, we extract a signed wrapper. + +```rust,ignore +struct Signed { + /// The payload is part of the signed data. The rest is the signing context, + /// which is known both at signing and at validation. + /// + /// Note that this field is not publicly accessible: this reduces the chances + /// of accidentally invalidating the signature by mutating the payload. + payload: Payload, + /// The index of the validator signing this statement. + pub validator_index: ValidatorIndex, + /// The signature by the validator of the signed payload. + pub signature: ValidatorSignature, +} + +impl, RealPayload> Signed { + fn sign(payload: Payload, context: SigningContext, index: ValidatorIndex, key: PrivateKey) -> Signed { ... } + fn validate(&self, context: SigningContext, key: PublicKey) -> bool { ... } + fn payload(&self) -> &Payload { &self.payload } +} +``` + +Note the presence of the [`SigningContext`](../types/candidate.html#signing-context) in the signatures of the `sign` and `validate` methods. To ensure cryptographic security, the actual signed payload is always the SCALE encoding of `(payload.into(), signing_context)`. Including the signing context prevents replay attacks. Converting the `T` into `U` is a NOP most of the time, but it allows us to substitute `CompactStatement`s for `Statement`s on demand, which helps efficiency. + ## Statement Type The [Candidate Backing subsystem](../node/backing/candidate-backing.html) issues and signs these after candidate validation. @@ -38,28 +65,36 @@ enum Statement { /// A statement about the invalidity of a candidate. Invalid(Hash), } + +/// A statement about the validity of a parachain candidate. +/// +/// This variant should only be used in the production of `SignedStatement`s. The only difference between +/// this enum and `Statement` is that the `Seconded` variant contains a `Hash` instead of a `CandidateReceipt`. +/// The rationale behind the difference is that a `CandidateReceipt` contains `HeadData`, which does not have +/// bounded size. By using this enum instead, we ensure that the production and validation of signatures is fast +/// while retaining their necessary cryptographic properties. +enum CompactStatement { + /// A statement about a new candidate being seconded by a validator. This is an implicit validity vote. + Seconded(Hash), + /// A statement about the validity of a candidate, based on candidate's hash. + Valid(Hash), + /// A statement about the invalidity of a candidate. + Invalid(Hash), +} ``` +`CompactStatement` exists because a `CandidateReceipt` includes `HeadData`, which does not have a bounded size. + ## Signed Statement Type -A statement, the identifier of a validator, and a signature. +A statement which has been [cryptographically signed](#signed-wrapper) by a validator. ```rust /// A signed statement. -struct SignedStatement { - /// The index of the validator signing this statement. - validator_index: ValidatorIndex, - /// The statement itself. - statement: Statement, - /// The signature by the validator on the signing payload. - signature: ValidatorSignature -} +pub type SignedStatement = Signed; ``` -The actual signed payload will be the SCALE encoding of `(compact_statement, signing_context)` where -`compact_statement` is a tweak of the [`Statement`](#statement) enum where all variants, including `Seconded`, contain only the hash of the candidate, and the `signing_context` is a [`SigningContext`](../types/candidate.html#signing-context). - -This prevents against replay attacks and allows the candidate receipt itself to be omitted when checking a signature on a `Seconded` statement in situations where the hash is known. +Munging the signed `Statement` into a `CompactStatement` before signing allows the candidate receipt itself to be omitted when checking a signature on a `Seconded` statement. ## Backed Candidate From fbec801ef9cb037ba0646dc7c44bbd8725322485 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Thu, 18 Jun 2020 13:47:03 +0200 Subject: [PATCH 02/11] typedef SignedAvailabilityBitfield --- .../implementors-guide/src/types/availability.md | 10 ++-------- roadmap/implementors-guide/src/types/backing.md | 14 +++++++------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/roadmap/implementors-guide/src/types/availability.md b/roadmap/implementors-guide/src/types/availability.md index 7556019eabcc..4fd3128513cf 100644 --- a/roadmap/implementors-guide/src/types/availability.md +++ b/roadmap/implementors-guide/src/types/availability.md @@ -5,21 +5,15 @@ candidates for the duration of a challenge period. This is done via an erasure-c ## Signed Availability Bitfield -A bitfield signed by a particular validator about the availability of pending candidates. +A bitfield [signed](backing.html#signed-wrapper) by a particular validator about the availability of pending candidates. ```rust -struct SignedAvailabilityBitfield { - validator_index: ValidatorIndex, - bitfield: Bitvec, - signature: ValidatorSignature, -} +pub type SignedAvailabilityBitfield = Signed; struct Bitfields(Vec<(SignedAvailabilityBitfield)>), // bitfields sorted by validator index, ascending ``` -The signed payload is the SCALE encoding of the tuple `(bitfield, signing_context)` where `signing_context` is a [`SigningContext`](../types/candidate.html#signing-context). - ## Proof-of-Validity Often referred to as PoV, this is a type-safe wrapper around bytes (`Vec`) when referring to data that acts as a stateless-client proof of validity of a candidate, when used as input to the validation function of the para. diff --git a/roadmap/implementors-guide/src/types/backing.md b/roadmap/implementors-guide/src/types/backing.md index f77cb450eed0..4b5b2fe0d88e 100644 --- a/roadmap/implementors-guide/src/types/backing.md +++ b/roadmap/implementors-guide/src/types/backing.md @@ -24,27 +24,27 @@ enum ValidityAttestation { There are a few distinct types which we desire to sign, and validate the signatures of. Instead of duplicating this work, we extract a signed wrapper. ```rust,ignore +/// A signed type which encapsulates the common desire to sign some data and validate a signature. +/// +/// Note that the internal fields are not public; they are all accessable by immutable getters. +/// This reduces the chance that they are accidentally mutated, invalidating the signature. struct Signed { /// The payload is part of the signed data. The rest is the signing context, /// which is known both at signing and at validation. - /// - /// Note that this field is not publicly accessible: this reduces the chances - /// of accidentally invalidating the signature by mutating the payload. payload: Payload, /// The index of the validator signing this statement. - pub validator_index: ValidatorIndex, + validator_index: ValidatorIndex, /// The signature by the validator of the signed payload. - pub signature: ValidatorSignature, + signature: ValidatorSignature, } impl, RealPayload> Signed { fn sign(payload: Payload, context: SigningContext, index: ValidatorIndex, key: PrivateKey) -> Signed { ... } fn validate(&self, context: SigningContext, key: PublicKey) -> bool { ... } - fn payload(&self) -> &Payload { &self.payload } } ``` -Note the presence of the [`SigningContext`](../types/candidate.html#signing-context) in the signatures of the `sign` and `validate` methods. To ensure cryptographic security, the actual signed payload is always the SCALE encoding of `(payload.into(), signing_context)`. Including the signing context prevents replay attacks. Converting the `T` into `U` is a NOP most of the time, but it allows us to substitute `CompactStatement`s for `Statement`s on demand, which helps efficiency. +Note the presence of the [`SigningContext`](../types/candidate.html#signing-context) in the signatures of the `sign` and `validate` methods. To ensure cryptographic security, the actual signed payload is always the SCALE encoding of `(payload.into(), signing_context)`. Including the signing context prevents replay attacks. Converting the `Payload` into `RealPayload` is a NOP most of the time, but it allows us to substitute `CompactStatement`s for `Statement`s on demand, which helps efficiency. ## Statement Type From 22eb23ace17d781ab97ad97734baef057a80e022 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Thu, 18 Jun 2020 15:22:20 +0200 Subject: [PATCH 03/11] implement Signed wrapper This is strictly an addition as of this commit; nothing is yet changed in existing behavior. --- Cargo.lock | 1 + node/primitives/Cargo.toml | 1 + node/primitives/src/lib.rs | 117 +++++++++++++++++- .../implementors-guide/src/types/backing.md | 10 +- 4 files changed, 120 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eb8ce72a765c..4525ee74c3c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4289,6 +4289,7 @@ dependencies = [ "parity-scale-codec", "polkadot-primitives", "polkadot-statement-table", + "sp-core", "sp-runtime", ] diff --git a/node/primitives/Cargo.toml b/node/primitives/Cargo.toml index f317565b2e99..3ee9307d3d48 100644 --- a/node/primitives/Cargo.toml +++ b/node/primitives/Cargo.toml @@ -10,3 +10,4 @@ polkadot-primitives = { path = "../../primitives" } polkadot-statement-table = { path = "../../statement-table" } parity-scale-codec = { version = "1.3.0", default-features = false, features = ["derive"] } runtime_primitives = { package = "sp-runtime", git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +sp-core = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } diff --git a/node/primitives/src/lib.rs b/node/primitives/src/lib.rs index 78a87cda39a9..1697256d7807 100644 --- a/node/primitives/src/lib.rs +++ b/node/primitives/src/lib.rs @@ -20,13 +20,14 @@ //! not shared between the node and the runtime. This crate builds on top of the primitives defined //! there. -use runtime_primitives::traits::AppVerify; -use polkadot_primitives::Hash; +use parity_scale_codec::{Decode, Encode}; use polkadot_primitives::parachain::{ - AbridgedCandidateReceipt, CandidateReceipt, SigningContext, ValidatorSignature, - ValidatorIndex, ValidatorId, + AbridgedCandidateReceipt, CandidateReceipt, SigningContext, ValidatorId, ValidatorIndex, + ValidatorPair, ValidatorSignature, }; -use parity_scale_codec::{Encode, Decode}; +use polkadot_primitives::Hash; +use runtime_primitives::traits::AppVerify; +use sp_core::crypto::Pair; /// A statement, where the candidate receipt is included in the `Seconded` variant. #[derive(Debug, Clone, PartialEq, Encode, Decode)] @@ -56,6 +57,112 @@ impl Statement { } } +/// This helper trait ensures that we can encode Statement as CompactStatement, +/// and anything as itself. +pub trait EncodeAs { + fn encode_as(&self) -> Vec; +} + +impl EncodeAs for T { + fn encode_as(&self) -> Vec { + self.encode() + } +} + +/// A statement about the validity of a parachain candidate. +/// +/// This variant should only be used in the production of `SignedStatement`s. The only difference between +/// this enum and `Statement` is that the `Seconded` variant contains a `Hash` instead of a `CandidateReceipt`. +/// The rationale behind the difference is that a `CandidateReceipt` contains `HeadData`, which does not have +/// bounded size. By using this enum instead, we ensure that the production and validation of signatures is fast +/// while retaining their necessary cryptographic properties. +#[derive(Debug, Clone, PartialEq, Encode, Decode)] +enum CompactStatement { + /// A statement about a new candidate being seconded by a validator. This is an implicit validity vote. + #[codec(index = "1")] + Seconded(Hash), + /// A statement about the validity of a candidate, based on candidate's hash. + #[codec(index = "2")] + Valid(Hash), + /// A statement about the invalidity of a candidate. + #[codec(index = "3")] + Invalid(Hash), +} + +impl EncodeAs for Statement { + fn encode_as(&self) -> Vec { + let statement = match *self { + Statement::Seconded(ref c) => { + polkadot_primitives::parachain::Statement::Candidate(c.hash()) + } + Statement::Valid(hash) => polkadot_primitives::parachain::Statement::Valid(hash), + Statement::Invalid(hash) => polkadot_primitives::parachain::Statement::Invalid(hash), + }; + statement.encode() + } +} + +/// A signed type which encapsulates the common desire to sign some data and validate a signature. +/// +/// Note that the internal fields are not public; they are all accessable by immutable getters. +/// This reduces the chance that they are accidentally mutated, invalidating the signature. +pub struct Signed { + /// The payload is part of the signed data. The rest is the signing context, + /// which is known both at signing and at validation. + payload: Payload, + /// The index of the validator signing this statement. + validator_index: ValidatorIndex, + /// The signature by the validator of the signed payload. + signature: ValidatorSignature, + /// This ensures the real payload is tracked at the typesystem level. + real_payload: std::marker::PhantomData, +} + +// We can't bound this on `Payload: Into` beacuse that conversion consumes +// the payload, and we don't want that. We can't bound it on `Payload: AsRef` +// because there's no blanket impl of `AsRef for T`. In the end, we just invent our +// own trait which does what we need: EncodeAs. +impl, RealPayload: Encode> Signed { + fn payload_data(payload: &Payload, context: SigningContext) -> Vec { + (payload.encode_as(), context).encode() + } + + pub fn sign( + payload: Payload, + context: SigningContext, + validator_index: ValidatorIndex, + key: &ValidatorPair, + ) -> Self { + let data = Self::payload_data(&payload, context); + let signature = key.sign(&data); + Self { + payload, + validator_index, + signature, + real_payload: std::marker::PhantomData, + } + } + + pub fn validate(&self, context: SigningContext, key: &ValidatorId) -> bool { + let data = Self::payload_data(&self.payload, context); + self.signature.verify(data.as_slice(), key) + } + + pub fn payload(&self) -> &Payload { + &self.payload + } + + // REVIEW: this works because ValidatorIndex is Copy, but that's dissimilar to the + // other getters. Keep it like this, or borrow it? + pub fn validator_index(&self) -> ValidatorIndex { + self.validator_index + } + + pub fn signature(&self) -> &ValidatorSignature { + &self.signature + } +} + /// A statement, the corresponding signature, and the index of the sender. /// /// Signing context and validator set should be apparent from context. diff --git a/roadmap/implementors-guide/src/types/backing.md b/roadmap/implementors-guide/src/types/backing.md index 4b5b2fe0d88e..a7a6fce6164b 100644 --- a/roadmap/implementors-guide/src/types/backing.md +++ b/roadmap/implementors-guide/src/types/backing.md @@ -38,13 +38,15 @@ struct Signed { signature: ValidatorSignature, } -impl, RealPayload> Signed { - fn sign(payload: Payload, context: SigningContext, index: ValidatorIndex, key: PrivateKey) -> Signed { ... } - fn validate(&self, context: SigningContext, key: PublicKey) -> bool { ... } +impl, RealPayload: Encode> Signed { + fn sign(payload: Payload, context: SigningContext, index: ValidatorIndex, key: ValidatorPair) -> Signed { ... } + fn validate(&self, context: SigningContext, key: ValidatorId) -> bool { ... } } ``` -Note the presence of the [`SigningContext`](../types/candidate.html#signing-context) in the signatures of the `sign` and `validate` methods. To ensure cryptographic security, the actual signed payload is always the SCALE encoding of `(payload.into(), signing_context)`. Including the signing context prevents replay attacks. Converting the `Payload` into `RealPayload` is a NOP most of the time, but it allows us to substitute `CompactStatement`s for `Statement`s on demand, which helps efficiency. +Note the presence of the [`SigningContext`](../types/candidate.html#signing-context) in the signatures of the `sign` and `validate` methods. To ensure cryptographic security, the actual signed payload is always the SCALE encoding of `(payload.into(), signing_context)`. Including the signing context prevents replay attacks. + +`EncodeAs` is a helper trait with a blanket impl which ensures that any `T` can `EncodeAs`. Therefore, for the generic case where `RealPayload = Payload`, it changes nothing. However, we `impl EncodeAs for Statement`, which helps efficiency. ## Statement Type From f974e48490efd8b382c50d0d3117f532916ecb71 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Thu, 18 Jun 2020 15:51:35 +0200 Subject: [PATCH 04/11] inline getters, remove review comment --- node/primitives/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/node/primitives/src/lib.rs b/node/primitives/src/lib.rs index 1697256d7807..43b450bd5c0f 100644 --- a/node/primitives/src/lib.rs +++ b/node/primitives/src/lib.rs @@ -148,16 +148,17 @@ impl, RealPayload: Encode> Signed &Payload { &self.payload } - // REVIEW: this works because ValidatorIndex is Copy, but that's dissimilar to the - // other getters. Keep it like this, or borrow it? + #[inline] pub fn validator_index(&self) -> ValidatorIndex { self.validator_index } + #[inline] pub fn signature(&self) -> &ValidatorSignature { &self.signature } From dde56ccfcab1f6a04a14b849428f61712e48c4e3 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Thu, 18 Jun 2020 17:08:08 +0200 Subject: [PATCH 05/11] move EncodeAs, Signed from node::primitives to primitives::parachain --- Cargo.lock | 1 - node/primitives/Cargo.toml | 1 - node/primitives/src/lib.rs | 154 +------------------------------ primitives/src/parachain.rs | 114 +++++++++++++++++++---- runtime/common/src/parachains.rs | 2 +- runtime/common/src/registrar.rs | 2 +- statement-table/src/lib.rs | 2 +- validation/src/lib.rs | 2 +- 8 files changed, 107 insertions(+), 171 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4525ee74c3c3..eb8ce72a765c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4289,7 +4289,6 @@ dependencies = [ "parity-scale-codec", "polkadot-primitives", "polkadot-statement-table", - "sp-core", "sp-runtime", ] diff --git a/node/primitives/Cargo.toml b/node/primitives/Cargo.toml index 3ee9307d3d48..f317565b2e99 100644 --- a/node/primitives/Cargo.toml +++ b/node/primitives/Cargo.toml @@ -10,4 +10,3 @@ polkadot-primitives = { path = "../../primitives" } polkadot-statement-table = { path = "../../statement-table" } parity-scale-codec = { version = "1.3.0", default-features = false, features = ["derive"] } runtime_primitives = { package = "sp-runtime", git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } -sp-core = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } diff --git a/node/primitives/src/lib.rs b/node/primitives/src/lib.rs index 43b450bd5c0f..1f84f47c5db7 100644 --- a/node/primitives/src/lib.rs +++ b/node/primitives/src/lib.rs @@ -21,13 +21,7 @@ //! there. use parity_scale_codec::{Decode, Encode}; -use polkadot_primitives::parachain::{ - AbridgedCandidateReceipt, CandidateReceipt, SigningContext, ValidatorId, ValidatorIndex, - ValidatorPair, ValidatorSignature, -}; -use polkadot_primitives::Hash; -use runtime_primitives::traits::AppVerify; -use sp_core::crypto::Pair; +use polkadot_primitives::{parachain::{AbridgedCandidateReceipt, CandidateReceipt, CompactStatement, EncodeAs, Signed}, Hash}; /// A statement, where the candidate receipt is included in the `Seconded` variant. #[derive(Debug, Clone, PartialEq, Encode, Decode)] @@ -43,161 +37,23 @@ pub enum Statement { Invalid(Hash), } -impl Statement { - /// Get the signing payload of the statement. - pub fn signing_payload(&self, context: &SigningContext) -> Vec { - // convert to fully hash-based payload. - let statement = match *self { - Statement::Seconded(ref c) => polkadot_primitives::parachain::Statement::Candidate(c.hash()), - Statement::Valid(hash) => polkadot_primitives::parachain::Statement::Valid(hash), - Statement::Invalid(hash) => polkadot_primitives::parachain::Statement::Invalid(hash), - }; - - statement.signing_payload(context) - } -} - -/// This helper trait ensures that we can encode Statement as CompactStatement, -/// and anything as itself. -pub trait EncodeAs { - fn encode_as(&self) -> Vec; -} - -impl EncodeAs for T { - fn encode_as(&self) -> Vec { - self.encode() - } -} - -/// A statement about the validity of a parachain candidate. -/// -/// This variant should only be used in the production of `SignedStatement`s. The only difference between -/// this enum and `Statement` is that the `Seconded` variant contains a `Hash` instead of a `CandidateReceipt`. -/// The rationale behind the difference is that a `CandidateReceipt` contains `HeadData`, which does not have -/// bounded size. By using this enum instead, we ensure that the production and validation of signatures is fast -/// while retaining their necessary cryptographic properties. -#[derive(Debug, Clone, PartialEq, Encode, Decode)] -enum CompactStatement { - /// A statement about a new candidate being seconded by a validator. This is an implicit validity vote. - #[codec(index = "1")] - Seconded(Hash), - /// A statement about the validity of a candidate, based on candidate's hash. - #[codec(index = "2")] - Valid(Hash), - /// A statement about the invalidity of a candidate. - #[codec(index = "3")] - Invalid(Hash), -} - impl EncodeAs for Statement { fn encode_as(&self) -> Vec { let statement = match *self { Statement::Seconded(ref c) => { - polkadot_primitives::parachain::Statement::Candidate(c.hash()) + polkadot_primitives::parachain::CompactStatement::Candidate(c.hash()) } - Statement::Valid(hash) => polkadot_primitives::parachain::Statement::Valid(hash), - Statement::Invalid(hash) => polkadot_primitives::parachain::Statement::Invalid(hash), + Statement::Valid(hash) => polkadot_primitives::parachain::CompactStatement::Valid(hash), + Statement::Invalid(hash) => polkadot_primitives::parachain::CompactStatement::Invalid(hash), }; statement.encode() } } -/// A signed type which encapsulates the common desire to sign some data and validate a signature. -/// -/// Note that the internal fields are not public; they are all accessable by immutable getters. -/// This reduces the chance that they are accidentally mutated, invalidating the signature. -pub struct Signed { - /// The payload is part of the signed data. The rest is the signing context, - /// which is known both at signing and at validation. - payload: Payload, - /// The index of the validator signing this statement. - validator_index: ValidatorIndex, - /// The signature by the validator of the signed payload. - signature: ValidatorSignature, - /// This ensures the real payload is tracked at the typesystem level. - real_payload: std::marker::PhantomData, -} - -// We can't bound this on `Payload: Into` beacuse that conversion consumes -// the payload, and we don't want that. We can't bound it on `Payload: AsRef` -// because there's no blanket impl of `AsRef for T`. In the end, we just invent our -// own trait which does what we need: EncodeAs. -impl, RealPayload: Encode> Signed { - fn payload_data(payload: &Payload, context: SigningContext) -> Vec { - (payload.encode_as(), context).encode() - } - - pub fn sign( - payload: Payload, - context: SigningContext, - validator_index: ValidatorIndex, - key: &ValidatorPair, - ) -> Self { - let data = Self::payload_data(&payload, context); - let signature = key.sign(&data); - Self { - payload, - validator_index, - signature, - real_payload: std::marker::PhantomData, - } - } - - pub fn validate(&self, context: SigningContext, key: &ValidatorId) -> bool { - let data = Self::payload_data(&self.payload, context); - self.signature.verify(data.as_slice(), key) - } - - #[inline] - pub fn payload(&self) -> &Payload { - &self.payload - } - - #[inline] - pub fn validator_index(&self) -> ValidatorIndex { - self.validator_index - } - - #[inline] - pub fn signature(&self) -> &ValidatorSignature { - &self.signature - } -} - /// A statement, the corresponding signature, and the index of the sender. /// /// Signing context and validator set should be apparent from context. -#[derive(Debug, Clone, PartialEq, Encode, Decode)] -pub struct SignedStatement { - /// The statement signed. - pub statement: Statement, - /// The signature of the validator. - pub signature: ValidatorSignature, - /// The index in the validator set of the signing validator. Which validator set should - /// be apparent from context. - pub sender: ValidatorIndex, -} - -impl SignedStatement { - /// Check the signature on a statement. Provide a list of validators to index into - /// and the context in which the statement is presumably signed. - /// - /// Returns an error if out of bounds or the signature is invalid. Otherwise, returns Ok. - pub fn check_signature( - &self, - validators: &[ValidatorId], - signing_context: &SigningContext, - ) -> Result<(), ()> { - let validator = validators.get(self.sender as usize).ok_or(())?; - let payload = self.statement.signing_payload(signing_context); - - if self.signature.verify(&payload[..], validator) { - Ok(()) - } else { - Err(()) - } - } -} +pub type SignedStatement = Signed; /// A misbehaviour report. pub enum MisbehaviorReport { diff --git a/primitives/src/parachain.rs b/primitives/src/parachain.rs index 195c986a23be..29fcc2a382ff 100644 --- a/primitives/src/parachain.rs +++ b/primitives/src/parachain.rs @@ -27,9 +27,9 @@ use super::{Hash, Balance, BlockNumber}; use serde::{Serialize, Deserialize}; #[cfg(feature = "std")] -use primitives::bytes; +use primitives::{bytes, crypto::Pair}; use primitives::RuntimeDebug; -use runtime_primitives::traits::Block as BlockT; +use runtime_primitives::traits::{AppVerify, Block as BlockT}; use inherents::InherentIdentifier; use application_crypto::KeyTypeId; @@ -245,8 +245,6 @@ fn check_collator_signature( collator: &CollatorId, signature: &CollatorSignature, ) -> Result<(),()> { - use runtime_primitives::traits::AppVerify; - let payload = collator_signature_payload(relay_parent, parachain_index, pov_block_hash); if signature.verify(&payload[..], collator) { Ok(()) @@ -581,7 +579,7 @@ pub struct Activity(#[cfg_attr(feature = "std", serde(with="bytes"))] pub Vec Vec { - (self, context).encode() - } -} - /// An either implicit or explicit attestation to the validity of a parachain /// candidate. #[derive(Clone, Eq, PartialEq, Decode, Encode, RuntimeDebug)] @@ -727,8 +716,6 @@ pub fn check_availability_bitfield_signature( signing_context: &SigningContext, payload_encode_buf: Option<&mut Vec>, ) -> Result<(),()> { - use runtime_primitives::traits::AppVerify; - let mut v = Vec::new(); let payload_encode_buf = payload_encode_buf.unwrap_or(&mut v); @@ -792,6 +779,101 @@ pub mod id { pub const PARACHAIN_HOST: ApiId = *b"parahost"; } +/// This helper trait ensures that we can encode Statement as CompactStatement, +/// and anything as itself. +/// +/// This resembles `parity_scale_codec::EncodeLike`, but it's distinct: +/// EncodeLike is a marker trait which asserts at the typesystem level that +/// one type's encoding is a valid encoding for another type. It doesn't +/// perform any type conversion when encoding. +/// +/// This trait, on the other hand, provides a method which can be used to +/// simultaneously convert and encode one type as another. +pub trait EncodeAs { + /// Convert Self into T, then encode T. + /// + /// This is useful when T is a subset of Self, reducing encoding costs; + /// its signature also means that we do not need to clone Self in order + /// to retain ownership, as we would if we were to do + /// `self.clone().into().encode()`. + fn encode_as(&self) -> Vec; +} + +impl EncodeAs for T { + fn encode_as(&self) -> Vec { + self.encode() + } +} + +/// A signed type which encapsulates the common desire to sign some data and validate a signature. +/// +/// Note that the internal fields are not public; they are all accessable by immutable getters. +/// This reduces the chance that they are accidentally mutated, invalidating the signature. +#[derive(Debug, Clone, PartialEq, Encode, Decode)] +pub struct Signed { + /// The payload is part of the signed data. The rest is the signing context, + /// which is known both at signing and at validation. + payload: Payload, + /// The index of the validator signing this statement. + validator_index: ValidatorIndex, + /// The signature by the validator of the signed payload. + signature: ValidatorSignature, + /// This ensures the real payload is tracked at the typesystem level. + real_payload: sp_std::marker::PhantomData, +} + +// We can't bound this on `Payload: Into` beacuse that conversion consumes +// the payload, and we don't want that. We can't bound it on `Payload: AsRef` +// because there's no blanket impl of `AsRef for T`. In the end, we just invent our +// own trait which does what we need: EncodeAs. +impl, RealPayload: Encode> Signed { + fn payload_data(payload: &Payload, context: SigningContext) -> Vec { + (payload.encode_as(), context).encode() + } + + /// Sign this payload with the given context and key, storing the validator index. + #[cfg(feature = "std")] + pub fn sign( + payload: Payload, + context: SigningContext, + validator_index: ValidatorIndex, + key: &ValidatorPair, + ) -> Self { + let data = Self::payload_data(&payload, context); + let signature = key.sign(&data); + Self { + payload, + validator_index, + signature, + real_payload: std::marker::PhantomData, + } + } + + /// Validate the payload given the context and public key. + pub fn validate(&self, context: SigningContext, key: &ValidatorId) -> bool { + let data = Self::payload_data(&self.payload, context); + self.signature.verify(data.as_slice(), key) + } + + /// Immutably access the payload. + #[inline] + pub fn payload(&self) -> &Payload { + &self.payload + } + + /// Immutably access the validator index. + #[inline] + pub fn validator_index(&self) -> ValidatorIndex { + self.validator_index + } + + /// Immutably access the signature. + #[inline] + pub fn signature(&self) -> &ValidatorSignature { + &self.signature + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 1f4d7621750e..ef67adb031f1 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -41,7 +41,7 @@ use primitives::{ Balance, BlockNumber, parachain::{ - Id as ParaId, Chain, DutyRoster, AttestedCandidate, Statement, ParachainDispatchOrigin, + Id as ParaId, Chain, DutyRoster, AttestedCandidate, CompactStatement as Statement, ParachainDispatchOrigin, UpwardMessage, ValidatorId, ActiveParas, CollatorId, Retriable, OmittedValidationData, CandidateReceipt, GlobalValidationSchedule, AbridgedCandidateReceipt, LocalValidationData, Scheduling, ValidityAttestation, NEW_HEADS_IDENTIFIER, PARACHAIN_KEY_TYPE_ID, diff --git a/runtime/common/src/registrar.rs b/runtime/common/src/registrar.rs index 5c31ec0f33e7..b5fbddb49eea 100644 --- a/runtime/common/src/registrar.rs +++ b/runtime/common/src/registrar.rs @@ -673,7 +673,7 @@ mod tests { use primitives::{ parachain::{ ValidatorId, Info as ParaInfo, Scheduling, LOWEST_USER_ID, AttestedCandidate, - CandidateReceipt, HeadData, ValidityAttestation, Statement, Chain, + CandidateReceipt, HeadData, ValidityAttestation, CompactStatement as Statement, Chain, CollatorPair, CandidateCommitments, }, Balance, BlockNumber, Header, Signature, diff --git a/statement-table/src/lib.rs b/statement-table/src/lib.rs index 16e8d9edfbc5..97d0cda76344 100644 --- a/statement-table/src/lib.rs +++ b/statement-table/src/lib.rs @@ -19,7 +19,7 @@ pub mod generic; pub use generic::Table; use primitives::parachain::{ - Id, AbridgedCandidateReceipt, Statement as PrimitiveStatement, ValidatorSignature, ValidatorIndex, + Id, AbridgedCandidateReceipt, CompactStatement as PrimitiveStatement, ValidatorSignature, ValidatorIndex, }; use primitives::Hash; diff --git a/validation/src/lib.rs b/validation/src/lib.rs index da5454a5ea7a..667f66e275a9 100644 --- a/validation/src/lib.rs +++ b/validation/src/lib.rs @@ -36,7 +36,7 @@ use std::{ use codec::Encode; use polkadot_primitives::parachain::{ Id as ParaId, Chain, DutyRoster, AbridgedCandidateReceipt, - Statement as PrimitiveStatement, + CompactStatement as PrimitiveStatement, PoVBlock, ErasureChunk, ValidatorSignature, ValidatorIndex, ValidatorPair, ValidatorId, SigningContext, }; From cffbc3e48acc195e479afbd4e68e73379c588c4e Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Thu, 18 Jun 2020 17:39:18 +0200 Subject: [PATCH 06/11] Refactor SignedAvailabilityBitfield to use Signed --- primitives/src/parachain.rs | 36 ++---------------------------------- 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/primitives/src/parachain.rs b/primitives/src/parachain.rs index 29fcc2a382ff..ca0932b8ad98 100644 --- a/primitives/src/parachain.rs +++ b/primitives/src/parachain.rs @@ -694,39 +694,7 @@ impl AvailabilityBitfield { } /// A bitfield signed by a particular validator about the availability of pending candidates. -#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] -pub struct SignedAvailabilityBitfield { - /// The index of the validator in the current set. - pub validator_index: ValidatorIndex, - /// The bitfield itself, with one bit per core. Only occupied cores may have the `1` bit set. - pub bitfield: AvailabilityBitfield, - /// The signature by the validator on the bitfield's signing payload. The context of the signature - /// should be apparent when checking the signature. - pub signature: ValidatorSignature, -} - -/// Check a signature on an availability bitfield. Provide the bitfield, the validator who signed it, -/// the signature, the signing context, and an optional buffer in which to encode. -/// -/// If the buffer is provided, it is assumed to be empty. -pub fn check_availability_bitfield_signature( - bitfield: &AvailabilityBitfield, - validator: &ValidatorId, - signature: &ValidatorSignature, - signing_context: &SigningContext, - payload_encode_buf: Option<&mut Vec>, -) -> Result<(),()> { - let mut v = Vec::new(); - let payload_encode_buf = payload_encode_buf.unwrap_or(&mut v); - - bitfield.encode_signing_payload_into(signing_context, payload_encode_buf); - - if signature.verify(&payload_encode_buf[..], validator) { - Ok(()) - } else { - Err(()) - } -} +pub type SignedAvailabilityBitfield = Signed; /// A set of signed availability bitfields. Should be sorted by validator index, ascending. #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] @@ -809,7 +777,7 @@ impl EncodeAs for T { /// /// Note that the internal fields are not public; they are all accessable by immutable getters. /// This reduces the chance that they are accidentally mutated, invalidating the signature. -#[derive(Debug, Clone, PartialEq, Encode, Decode)] +#[derive(Debug, Clone, PartialEq, Eq, Encode, Decode)] pub struct Signed { /// The payload is part of the signed data. The rest is the signing context, /// which is known both at signing and at validation. From 234eca55082d94886fa70a440ba9281bdce31a60 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Fri, 19 Jun 2020 10:34:38 +0200 Subject: [PATCH 07/11] don't double-encode real payload This isn't an ideal solution, because it depends on the implementation details of how SCALE encodes tuples, but OTOH that behavior seems unlikely to change anytime soon. --- primitives/src/parachain.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/primitives/src/parachain.rs b/primitives/src/parachain.rs index ca0932b8ad98..26c221dda906 100644 --- a/primitives/src/parachain.rs +++ b/primitives/src/parachain.rs @@ -796,7 +796,10 @@ pub struct Signed { // own trait which does what we need: EncodeAs. impl, RealPayload: Encode> Signed { fn payload_data(payload: &Payload, context: SigningContext) -> Vec { - (payload.encode_as(), context).encode() + // equivalent to (real_payload, context).encode() + let mut out = payload.encode_as(); + out.extend(context.encode()); + out } /// Sign this payload with the given context and key, storing the validator index. From d886979cc394a4220dcc3617439855305b8c83e0 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Fri, 19 Jun 2020 12:38:24 +0200 Subject: [PATCH 08/11] fix build errors --- primitives/src/parachain.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/primitives/src/parachain.rs b/primitives/src/parachain.rs index 624622d23988..5b84939ab4ed 100644 --- a/primitives/src/parachain.rs +++ b/primitives/src/parachain.rs @@ -636,11 +636,11 @@ impl ValidityAttestation { ) -> Vec { match *self { ValidityAttestation::Implicit(_) => ( - Statement::Candidate(candidate_hash), + CompactStatement::Candidate(candidate_hash), signing_context, ).encode(), ValidityAttestation::Explicit(_) => ( - Statement::Valid(candidate_hash), + CompactStatement::Valid(candidate_hash), signing_context, ).encode(), } @@ -771,8 +771,6 @@ pub fn check_candidate_backing + Encode>( group_len: usize, validator_lookup: impl Fn(usize) -> Option, ) -> Result { - use runtime_primitives::traits::AppVerify; - if backed.validator_indices.len() != group_len { return Err(()) } @@ -914,9 +912,9 @@ impl, RealPayload: Encode> Signed bool { + pub fn check_signature(&self, context: SigningContext, key: &ValidatorId) -> Result<(), ()> { let data = Self::payload_data(&self.payload, context); - self.signature.verify(data.as_slice(), key) + if self.signature.verify(data.as_slice(), key) { Ok(()) } else { Err(()) } } /// Immutably access the payload. From 8791b2c3bbc533e2d1583fd9078fffcbf24a34df Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Fri, 19 Jun 2020 14:04:12 +0200 Subject: [PATCH 09/11] cause the runtime to build properly with the new changes Not sure why cargo check didn't catch this earlier; oh well. --- primitives/src/parachain.rs | 17 ++++++++++---- runtime/parachains/src/inclusion.rs | 35 +++++++++++------------------ 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/primitives/src/parachain.rs b/primitives/src/parachain.rs index 5b84939ab4ed..afaac96d2807 100644 --- a/primitives/src/parachain.rs +++ b/primitives/src/parachain.rs @@ -886,7 +886,7 @@ pub struct Signed { // because there's no blanket impl of `AsRef for T`. In the end, we just invent our // own trait which does what we need: EncodeAs. impl, RealPayload: Encode> Signed { - fn payload_data(payload: &Payload, context: SigningContext) -> Vec { + fn payload_data(payload: &Payload, context: &SigningContext) -> Vec { // equivalent to (real_payload, context).encode() let mut out = payload.encode_as(); out.extend(context.encode()); @@ -895,9 +895,9 @@ impl, RealPayload: Encode> Signed( payload: Payload, - context: SigningContext, + context: &SigningContext, validator_index: ValidatorIndex, key: &ValidatorPair, ) -> Self { @@ -912,7 +912,7 @@ impl, RealPayload: Encode> Signed Result<(), ()> { + pub fn check_signature(&self, context: &SigningContext, key: &ValidatorId) -> Result<(), ()> { let data = Self::payload_data(&self.payload, context); if self.signature.verify(data.as_slice(), key) { Ok(()) } else { Err(()) } } @@ -934,6 +934,15 @@ impl, RealPayload: Encode> Signed &ValidatorSignature { &self.signature } + + /// Discard signing data, get the payload + // Note: can't `impl From> for P` because the orphan rule exception doesn't + // handle this case yet. Likewise can't `impl Into

for Signed` because it might + // potentially conflict with the global blanket impl, even though it currently doesn't. + #[inline] + pub fn into_payload(self) -> Payload { + self.payload + } } #[cfg(test)] diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index 5fba4cf26ca3..de666acfd9b8 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -184,7 +184,6 @@ impl Module { .collect(); let mut last_index = None; - let mut payload_encode_buf = Vec::new(); let signing_context = SigningContext { parent_hash: >::parent_hash(), @@ -193,46 +192,37 @@ impl Module { for signed_bitfield in &signed_bitfields.0 { ensure!( - signed_bitfield.bitfield.0.len() == n_bits, + signed_bitfield.payload().0.len() == n_bits, Error::::WrongBitfieldSize, ); ensure!( - last_index.map_or(true, |last| last < signed_bitfield.validator_index), + last_index.map_or(true, |last| last < signed_bitfield.validator_index()), Error::::BitfieldDuplicateOrUnordered, ); ensure!( - signed_bitfield.validator_index < validators.len() as ValidatorIndex, + signed_bitfield.validator_index() < validators.len() as ValidatorIndex, Error::::ValidatorIndexOutOfBounds, ); ensure!( - occupied_bitmask.clone() & signed_bitfield.bitfield.0.clone() == signed_bitfield.bitfield.0, + occupied_bitmask.clone() & signed_bitfield.payload().0.clone() == signed_bitfield.payload().0, Error::::UnoccupiedBitInBitfield, ); - let validator_public = &validators[signed_bitfield.validator_index as usize]; + let validator_public = &validators[signed_bitfield.validator_index() as usize]; - if let Err(()) = primitives::parachain::check_availability_bitfield_signature( - &signed_bitfield.bitfield, - validator_public, - &signed_bitfield.signature, - &signing_context, - Some(&mut payload_encode_buf), - ) { - Err(Error::::InvalidBitfieldSignature)?; - } + signed_bitfield.check_signature(&signing_context, validator_public).map_err(|_| Error::::InvalidBitfieldSignature)?; - last_index = Some(signed_bitfield.validator_index); - payload_encode_buf.clear(); + last_index = Some(signed_bitfield.validator_index()); } } let now = >::block_number(); for signed_bitfield in signed_bitfields.0 { for (bit_idx, _) - in signed_bitfield.bitfield.0.iter().enumerate().filter(|(_, is_av)| **is_av) + in signed_bitfield.payload().0.iter().enumerate().filter(|(_, is_av)| **is_av) { let record = assigned_paras_record[bit_idx] .as_mut() @@ -240,7 +230,7 @@ impl Module { // defensive check - this is constructed by loading the availability bitfield record, // which is always `Some` if the core is occupied - that's why we're here. - let val_idx = signed_bitfield.validator_index as usize; + let val_idx = signed_bitfield.validator_index() as usize; if let Some(mut bit) = record.1.as_mut() .and_then(|r| r.availability_votes.get_mut(val_idx)) { @@ -250,12 +240,13 @@ impl Module { } } + let validator_index = signed_bitfield.validator_index(); let record = AvailabilityBitfieldRecord { - bitfield: signed_bitfield.bitfield, + bitfield: signed_bitfield.into_payload(), submitted_at: now, }; - >::insert(&signed_bitfield.validator_index, record); + >::insert(&validator_index, record); } let threshold = availability_threshold(validators.len()); @@ -506,7 +497,7 @@ mod tests { use primitives::{BlockNumber, Hash}; use primitives::parachain::{ - SignedAvailabilityBitfield, Statement, ValidityAttestation, CollatorId, + SignedAvailabilityBitfield, CompactStatement as Statement, ValidityAttestation, CollatorId, CandidateCommitments, }; use frame_support::traits::{OnFinalize, OnInitialize}; From 75ce5291a6ec058fdf32fa6b43d9ba2b0fe26ab1 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 19 Jun 2020 15:24:24 -0400 Subject: [PATCH 10/11] fix runtime tests and separate SignedStatement from SignedFullStatement --- node/messages/src/lib.rs | 6 ++--- node/primitives/src/lib.rs | 18 +++++++++---- primitives/src/parachain.rs | 25 +++---------------- .../node/backing/statement-distribution.md | 6 ++--- .../implementors-guide/src/types/backing.md | 7 ++++-- .../src/types/overseer-protocol.md | 10 ++++---- runtime/parachains/src/inclusion.rs | 23 ++++++++++------- 7 files changed, 46 insertions(+), 49 deletions(-) diff --git a/node/messages/src/lib.rs b/node/messages/src/lib.rs index 4d6da67d84a2..3a413f2c67bb 100644 --- a/node/messages/src/lib.rs +++ b/node/messages/src/lib.rs @@ -31,7 +31,7 @@ use polkadot_primitives::parachain::{ SignedAvailabilityBitfield, SigningContext, ValidatorId, ValidationCode, ValidatorIndex, }; use polkadot_node_primitives::{ - MisbehaviorReport, SignedStatement, + MisbehaviorReport, SignedFullStatement, }; /// Signals sent by an overseer to a subsystem. @@ -68,7 +68,7 @@ pub enum CandidateBackingMessage { Second(Hash, AbridgedCandidateReceipt), /// Note a validator's statement about a particular candidate. Disagreements about validity must be escalated /// to a broader check by Misbehavior Arbitration. Agreements are simply tallied until a quorum is reached. - Statement(Hash, SignedStatement), + Statement(Hash, SignedFullStatement), } /// Blanket error for validation failing. @@ -180,7 +180,7 @@ pub enum RuntimeApiMessage { pub enum StatementDistributionMessage { /// We have originated a signed statement in the context of /// given relay-parent hash and it should be distributed to other validators. - Share(Hash, SignedStatement), + Share(Hash, SignedFullStatement), } /// This data becomes intrinsics or extrinsics which should be included in a future relay chain block. diff --git a/node/primitives/src/lib.rs b/node/primitives/src/lib.rs index 1f84f47c5db7..bd43748ab24a 100644 --- a/node/primitives/src/lib.rs +++ b/node/primitives/src/lib.rs @@ -21,7 +21,12 @@ //! there. use parity_scale_codec::{Decode, Encode}; -use polkadot_primitives::{parachain::{AbridgedCandidateReceipt, CandidateReceipt, CompactStatement, EncodeAs, Signed}, Hash}; +use polkadot_primitives::{Hash, + parachain::{ + AbridgedCandidateReceipt, CandidateReceipt, CompactStatement, + EncodeAs, Signed, + } +}; /// A statement, where the candidate receipt is included in the `Seconded` variant. #[derive(Debug, Clone, PartialEq, Encode, Decode)] @@ -53,7 +58,10 @@ impl EncodeAs for Statement { /// A statement, the corresponding signature, and the index of the sender. /// /// Signing context and validator set should be apparent from context. -pub type SignedStatement = Signed; +/// +/// This statement is "full" in the sense that the `Seconded` variant includes the candidate receipt. +/// Only the compact `SignedStatement` is suitable for submission to the chain. +pub type SignedFullStatement = Signed; /// A misbehaviour report. pub enum MisbehaviorReport { @@ -65,9 +73,9 @@ pub enum MisbehaviorReport { /// this message should be dispatched with all of them, in arbitrary order. /// /// This variant is also used when our own validity checks disagree with others'. - CandidateValidityDisagreement(CandidateReceipt, Vec), + CandidateValidityDisagreement(CandidateReceipt, Vec), /// I've noticed a peer contradicting itself about a particular candidate - SelfContradiction(CandidateReceipt, SignedStatement, SignedStatement), + SelfContradiction(CandidateReceipt, SignedFullStatement, SignedFullStatement), /// This peer has seconded more than one parachain candidate for this relay parent head - DoubleVote(CandidateReceipt, SignedStatement, SignedStatement), + DoubleVote(CandidateReceipt, SignedFullStatement, SignedFullStatement), } diff --git a/primitives/src/parachain.rs b/primitives/src/parachain.rs index afaac96d2807..a24cb5b612de 100644 --- a/primitives/src/parachain.rs +++ b/primitives/src/parachain.rs @@ -604,6 +604,9 @@ pub enum CompactStatement { Invalid(Hash), } +/// A signed compact statement, suitable to be sent to the chain. +pub type SignedStatement = Signed; + /// An either implicit or explicit attestation to the validity of a parachain /// candidate. #[derive(Clone, Eq, PartialEq, Decode, Encode, RuntimeDebug)] @@ -712,28 +715,6 @@ impl From> for AvailabilityBitfield { } } -impl AvailabilityBitfield { - /// Encodes the signing payload into the given buffer. - pub fn encode_signing_payload_into( - &self, - signing_context: &SigningContext, - buf: &mut Vec, - ) { - self.0.encode_to(buf); - signing_context.encode_to(buf); - } - - /// Encodes the signing payload into a fresh byte-vector. - pub fn encode_signing_payload( - &self, - signing_context: &SigningContext, - ) -> Vec { - let mut v = Vec::new(); - self.encode_signing_payload_into(signing_context, &mut v); - v - } -} - /// A bitfield signed by a particular validator about the availability of pending candidates. pub type SignedAvailabilityBitfield = Signed; diff --git a/roadmap/implementors-guide/src/node/backing/statement-distribution.md b/roadmap/implementors-guide/src/node/backing/statement-distribution.md index 1683361c8554..b2a30a712039 100644 --- a/roadmap/implementors-guide/src/node/backing/statement-distribution.md +++ b/roadmap/implementors-guide/src/node/backing/statement-distribution.md @@ -35,11 +35,11 @@ The Statement Distribution subsystem sends statements to peer nodes and detects ## Peer Receipt State Machine -There is a very simple state machine which governs which messages we are willing to receive from peers. Not depicted in the state machine: on initial receipt of any [`SignedStatement`](../../types/backing.html#signed-statement-type), validate that the provided signature does in fact sign the included data. Note that each individual parablock candidate gets its own instance of this state machine; it is perfectly legal to receive a `Valid(X)` before a `Seconded(Y)`, as long as a `Seconded(X)` has been received. +There is a very simple state machine which governs which messages we are willing to receive from peers. Not depicted in the state machine: on initial receipt of any [`SignedFullStatement`](../../types/backing.html#signed-statement-type), validate that the provided signature does in fact sign the included data. Note that each individual parablock candidate gets its own instance of this state machine; it is perfectly legal to receive a `Valid(X)` before a `Seconded(Y)`, as long as a `Seconded(X)` has been received. -A: Initial State. Receive `SignedStatement(Statement::Second)`: extract `Statement`, forward to Candidate Backing, proceed to B. Receive any other `SignedStatement` variant: drop it. +A: Initial State. Receive `SignedFullStatement(Statement::Second)`: extract `Statement`, forward to Candidate Backing, proceed to B. Receive any other `SignedFullStatement` variant: drop it. -B: Receive any `SignedStatement`: check signature, forward to Candidate Backing. Receive `OverseerMessage::StopWork`: proceed to C. +B: Receive any `SignedFullStatement`: check signature, forward to Candidate Backing. Receive `OverseerMessage::StopWork`: proceed to C. C: Receive any message for this block: drop it. diff --git a/roadmap/implementors-guide/src/types/backing.md b/roadmap/implementors-guide/src/types/backing.md index a7a6fce6164b..5b61522b3375 100644 --- a/roadmap/implementors-guide/src/types/backing.md +++ b/roadmap/implementors-guide/src/types/backing.md @@ -92,8 +92,11 @@ enum CompactStatement { A statement which has been [cryptographically signed](#signed-wrapper) by a validator. ```rust -/// A signed statement. -pub type SignedStatement = Signed; +/// A signed statement, containing the abridged candidate receipt in the `Seconded` variant. +pub type SignedFullStatement = Signed; + +/// A signed statement, containing only the hash. +pub type SignedStatement = Signed; ``` Munging the signed `Statement` into a `CompactStatement` before signing allows the candidate receipt itself to be omitted when checking a signature on a `Seconded` statement. diff --git a/roadmap/implementors-guide/src/types/overseer-protocol.md b/roadmap/implementors-guide/src/types/overseer-protocol.md index f00583e3d9a3..94c4e5510277 100644 --- a/roadmap/implementors-guide/src/types/overseer-protocol.md +++ b/roadmap/implementors-guide/src/types/overseer-protocol.md @@ -158,11 +158,11 @@ enum MisbehaviorReport { /// this message should be dispatched with all of them, in arbitrary order. /// /// This variant is also used when our own validity checks disagree with others'. - CandidateValidityDisagreement(CandidateReceipt, Vec), + CandidateValidityDisagreement(CandidateReceipt, Vec), /// I've noticed a peer contradicting itself about a particular candidate - SelfContradiction(CandidateReceipt, SignedStatement, SignedStatement), + SelfContradiction(CandidateReceipt, SignedFullStatement, SignedFullStatement), /// This peer has seconded more than one parachain candidate for this relay parent head - DoubleVote(CandidateReceipt, SignedStatement, SignedStatement), + DoubleVote(CandidateReceipt, SignedFullStatement, SignedFullStatement), } ``` @@ -227,7 +227,7 @@ enum RuntimeApiMessage { ## Statement Distribution Message -The Statement Distribution subsystem distributes signed statements from validators to other validators. +The Statement Distribution subsystem distributes signed statements and candidates from validators to other validators. It does this by distributing full statements, which embed the candidate receipt, as opposed to compact statements which don't. It receives updates from the network bridge and signed statements to share with other validators. ```rust @@ -239,7 +239,7 @@ enum StatementDistributionMessage { /// /// The statement distribution subsystem assumes that the statement should be correctly /// signed. - Share(Hash, SignedStatement), + Share(Hash, SignedFullStatement), } ``` diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index de666acfd9b8..c31c4a62e6e2 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -498,7 +498,7 @@ mod tests { use primitives::{BlockNumber, Hash}; use primitives::parachain::{ SignedAvailabilityBitfield, CompactStatement as Statement, ValidityAttestation, CollatorId, - CandidateCommitments, + CandidateCommitments, SignedStatement, }; use frame_support::traits::{OnFinalize, OnInitialize}; use keyring::Sr25519Keyring; @@ -578,13 +578,19 @@ mod tests { let mut validity_votes = Vec::with_capacity(signing); let candidate_hash = candidate.hash(); - let payload = Statement::Valid(candidate_hash).signing_payload(signing_context); for (idx_in_group, val_idx) in group.iter().enumerate().take(signing) { let key: Sr25519Keyring = validators[*val_idx as usize]; *validator_indices.get_mut(idx_in_group).unwrap() = true; - validity_votes.push(ValidityAttestation::Explicit(key.sign(&payload[..]).into())); + let signature = SignedStatement::sign( + Statement::Valid(candidate_hash), + signing_context, + *val_idx, + &key.pair().into(), + ).signature().clone(); + + validity_votes.push(ValidityAttestation::Explicit(signature).into()); } let backed = BackedCandidate { @@ -661,13 +667,12 @@ mod tests { ) -> SignedAvailabilityBitfield { - let payload = bitfield.encode_signing_payload(signing_context); - - SignedAvailabilityBitfield { + SignedAvailabilityBitfield::sign( + bitfield, + &signing_context, validator_index, - bitfield: bitfield, - signature: key.sign(&payload[..]).into(), - } + &key.pair().into(), + ) } #[test] From 46ebb8a0223049d80d983ff21d5ffc02ae71d43f Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Sat, 20 Jun 2020 12:07:47 +0200 Subject: [PATCH 11/11] better explain why CompactStatement exists Co-authored-by: Robert Habermeier --- roadmap/implementors-guide/src/types/backing.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/roadmap/implementors-guide/src/types/backing.md b/roadmap/implementors-guide/src/types/backing.md index 5b61522b3375..2cb3118937f9 100644 --- a/roadmap/implementors-guide/src/types/backing.md +++ b/roadmap/implementors-guide/src/types/backing.md @@ -72,9 +72,8 @@ enum Statement { /// /// This variant should only be used in the production of `SignedStatement`s. The only difference between /// this enum and `Statement` is that the `Seconded` variant contains a `Hash` instead of a `CandidateReceipt`. -/// The rationale behind the difference is that a `CandidateReceipt` contains `HeadData`, which does not have -/// bounded size. By using this enum instead, we ensure that the production and validation of signatures is fast -/// while retaining their necessary cryptographic properties. +/// The rationale behind the difference is that the signature should always be on the hash instead of the +/// full data, as this lowers the requirement for checking while retaining necessary cryptographic properties enum CompactStatement { /// A statement about a new candidate being seconded by a validator. This is an implicit validity vote. Seconded(Hash),