Unverified Commit fff46359 authored by asynchronous rob's avatar asynchronous rob Committed by GitHub
Browse files

Move erasure root out of candidate commitments and into descriptor (#2010)



* guide: move erasure-root to candidate descriptor

* primitives: move erasure root to descriptor

* guide: unify candidate commitments and validation outputs

* primitives: unify validation outputs and candidate commitments

* parachains-runtime: fix fallout

* runtimes: fix fallout

* collation generation: fix fallout

* fix stray reference in primitives

* fix fallout in node-primitives

* fix remaining fallout in collation generation

* fix fallout in candidate validation

* fix fallout in runtime API subsystem

* fix fallout in subsystem messages

* fix fallout in candidate backing

* fix fallout in availability distribution

* don't clone

* clone
Co-authored-by: Sergey Pepyakin's avatarSergei Shulepov <sergei@parity.io>
parent 1a1f858f
Pipeline #115325 passed with stages
in 29 minutes and 18 seconds
......@@ -284,7 +284,6 @@ async fn handle_new_activations<Context: SubsystemContext>(
horizontal_messages: collation.horizontal_messages,
new_validation_code: collation.new_validation_code,
head_data: collation.head_data,
erasure_root,
processed_downward_messages: collation.processed_downward_messages,
hrmp_watermark: collation.hrmp_watermark,
};
......@@ -298,6 +297,7 @@ async fn handle_new_activations<Context: SubsystemContext>(
collator: task_config.key.public(),
persisted_validation_data_hash,
pov_hash,
erasure_root,
},
};
......@@ -702,6 +702,7 @@ mod tests {
collator: config.key.public(),
persisted_validation_data_hash: expect_validation_data_hash,
pov_hash: expect_pov_hash,
erasure_root: Default::default(), // this isn't something we're checking right now
};
assert_eq!(sent_messages.len(), 1);
......@@ -728,6 +729,7 @@ mod tests {
let expect_descriptor = {
let mut expect_descriptor = expect_descriptor;
expect_descriptor.signature = descriptor.signature.clone();
expect_descriptor.erasure_root = descriptor.erasure_root.clone();
expect_descriptor
};
assert_eq!(descriptor, &expect_descriptor);
......
......@@ -31,8 +31,7 @@ use polkadot_primitives::v1::{
CommittedCandidateReceipt, BackedCandidate, Id as ParaId, ValidatorId,
ValidatorIndex, SigningContext, PoV, CandidateHash,
CandidateDescriptor, AvailableData, ValidatorSignature, Hash, CandidateReceipt,
CandidateCommitments, CoreState, CoreIndex, CollatorId, ValidationOutputs,
ValidityAttestation,
CoreState, CoreIndex, CollatorId, ValidityAttestation,
};
use polkadot_node_primitives::{
FromTableMisbehavior, Statement, SignedFullStatement, MisbehaviorReport, ValidationResult,
......@@ -229,6 +228,8 @@ impl TryFrom<AllMessages> for FromJob {
}
}
struct InvalidErasureRoot;
// It looks like it's not possible to do an `impl From` given the current state of
// the code. So this does the necessary conversion.
fn primitive_statement_to_table(s: &SignedFullStatement) -> TableSignedStatement {
......@@ -347,36 +348,38 @@ impl CandidateBackingJob {
let candidate_hash = candidate.hash();
let statement = match valid {
ValidationResult::Valid(outputs, validation_data) => {
ValidationResult::Valid(commitments, validation_data) => {
// make PoV available for later distribution. Send data to the availability
// store to keep. Sign and dispatch `valid` statement to network if we
// have not seconded the given candidate.
//
// If the commitments hash produced by validation is not the same as given by
// the collator, do not make available and report the collator.
let commitments_check = self.make_pov_available(
pov,
candidate_hash,
validation_data,
outputs,
|commitments| if commitments.hash() == candidate.commitments_hash {
Ok(CommittedCandidateReceipt {
descriptor: candidate.descriptor().clone(),
commitments,
})
} else {
Err(())
},
).await?;
match commitments_check {
Ok(candidate) => {
self.issued_statements.insert(candidate_hash);
Some(Statement::Seconded(candidate))
}
Err(()) => {
self.issue_candidate_invalid_message(candidate.clone()).await?;
None
if candidate.commitments_hash != commitments.hash() {
self.issue_candidate_invalid_message(candidate.clone()).await?;
None
} else {
let erasure_valid = self.make_pov_available(
pov,
candidate_hash,
validation_data,
candidate.descriptor.erasure_root,
).await?;
match erasure_valid {
Ok(()) => {
let candidate = CommittedCandidateReceipt {
descriptor: candidate.descriptor().clone(),
commitments,
};
self.issued_statements.insert(candidate_hash);
Some(Statement::Seconded(candidate))
}
Err(InvalidErasureRoot) => {
self.issue_candidate_invalid_message(candidate.clone()).await?;
None
}
}
}
}
......@@ -566,6 +569,7 @@ impl CandidateBackingJob {
// and not just those things that the function uses.
let candidate = self.table.get_candidate(&candidate_hash).ok_or(Error::CandidateNotFound)?;
let expected_commitments = candidate.commitments.clone();
let expected_erasure_root = candidate.descriptor.erasure_root;
let descriptor = candidate.descriptor().clone();
......@@ -585,23 +589,22 @@ impl CandidateBackingJob {
let v = self.request_candidate_validation(descriptor, pov.clone()).await?;
let statement = match v {
ValidationResult::Valid(outputs, validation_data) => {
ValidationResult::Valid(commitments, validation_data) => {
// If validation produces a new set of commitments, we vote the candidate as invalid.
let commitments_check = self.make_pov_available(
pov,
candidate_hash,
validation_data,
outputs,
|commitments| if commitments == expected_commitments {
Ok(())
} else {
Err(())
if commitments != expected_commitments {
Statement::Invalid(candidate_hash)
} else {
let erasure_valid = self.make_pov_available(
pov,
candidate_hash,
validation_data,
expected_erasure_root,
).await?;
match erasure_valid {
Ok(()) => Statement::Valid(candidate_hash),
Err(InvalidErasureRoot) => Statement::Invalid(candidate_hash),
}
).await?;
match commitments_check {
Ok(()) => Statement::Valid(candidate_hash),
Err(()) => Statement::Invalid(candidate_hash),
}
}
ValidationResult::Invalid(_reason) => {
......@@ -733,18 +736,16 @@ impl CandidateBackingJob {
// Make a `PoV` available.
//
// This calls an inspection function before making the PoV available for any last checks
// that need to be done. If the inspection function returns an error, this function returns
// early without making the PoV available.
#[tracing::instrument(level = "trace", skip(self, pov, with_commitments), fields(subsystem = LOG_TARGET))]
async fn make_pov_available<T, E>(
// This will compute the erasure root internally and compare it to the expected erasure root.
// This returns `Err()` iff there is an internal error. Otherwise, it returns either `Ok(Ok(()))` or `Ok(Err(_))`.
#[tracing::instrument(level = "trace", skip(self, pov), fields(subsystem = LOG_TARGET))]
async fn make_pov_available(
&mut self,
pov: Arc<PoV>,
candidate_hash: CandidateHash,
validation_data: polkadot_primitives::v1::PersistedValidationData,
outputs: ValidationOutputs,
with_commitments: impl FnOnce(CandidateCommitments) -> Result<T, E>,
) -> Result<Result<T, E>, Error> {
expected_erasure_root: Hash,
) -> Result<Result<(), InvalidErasureRoot>, Error> {
let available_data = AvailableData {
pov,
validation_data,
......@@ -758,20 +759,9 @@ impl CandidateBackingJob {
let branches = erasure_coding::branches(chunks.as_ref());
let erasure_root = branches.root();
let commitments = CandidateCommitments {
upward_messages: outputs.upward_messages,
horizontal_messages: outputs.horizontal_messages,
erasure_root,
new_validation_code: outputs.new_validation_code,
head_data: outputs.head_data,
processed_downward_messages: outputs.processed_downward_messages,
hrmp_watermark: outputs.hrmp_watermark,
};
let res = match with_commitments(commitments) {
Ok(x) => x,
Err(e) => return Ok(Err(e)),
};
if erasure_root != expected_erasure_root {
return Ok(Err(InvalidErasureRoot));
}
self.store_available_data(
self.table_context.validator.as_ref().map(|v| v.index()),
......@@ -780,7 +770,7 @@ impl CandidateBackingJob {
available_data,
).await?;
Ok(Ok(res))
Ok(Ok(()))
}
async fn distribute_signed_statement(&mut self, s: SignedFullStatement) -> Result<(), Error> {
......@@ -1183,11 +1173,11 @@ mod tests {
para_id: self.para_id,
pov_hash: self.pov_hash,
relay_parent: self.relay_parent,
erasure_root: self.erasure_root,
..Default::default()
},
commitments: CandidateCommitments {
head_data: self.head_data,
erasure_root: self.erasure_root,
..Default::default()
},
}
......@@ -1290,7 +1280,7 @@ mod tests {
)
) if pov == pov && &c == candidate.descriptor() => {
tx.send(Ok(
ValidationResult::Valid(ValidationOutputs {
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
horizontal_messages: Vec::new(),
upward_messages: Vec::new(),
......@@ -1428,7 +1418,7 @@ mod tests {
)
) if pov == pov && &c == candidate_a.descriptor() => {
tx.send(Ok(
ValidationResult::Valid(ValidationOutputs {
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
......@@ -1579,7 +1569,7 @@ mod tests {
)
) if pov == pov && &c == candidate_a.descriptor() => {
tx.send(Ok(
ValidationResult::Valid(ValidationOutputs {
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
......@@ -1764,7 +1754,7 @@ mod tests {
)
) if pov == pov && &c == candidate_b.descriptor() => {
tx.send(Ok(
ValidationResult::Valid(ValidationOutputs {
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
......
......@@ -36,7 +36,7 @@ use polkadot_subsystem::errors::RuntimeApiError;
use polkadot_node_primitives::{ValidationResult, InvalidCandidate};
use polkadot_primitives::v1::{
ValidationCode, PoV, CandidateDescriptor, PersistedValidationData,
OccupiedCoreAssumption, Hash, ValidationOutputs,
OccupiedCoreAssumption, Hash, CandidateCommitments,
};
use polkadot_parachain::wasm_executor::{
self, IsolationStrategy, ValidationError, InvalidCandidate as WasmInvalidCandidate
......@@ -458,7 +458,7 @@ fn validate_candidate_exhaustive<B: ValidationBackend, S: SpawnNamed + 'static>(
Ok(ValidationResult::Invalid(InvalidCandidate::ExecutionError(e.to_string()))),
Err(ValidationError::Internal(e)) => Err(ValidationFailed(e.to_string())),
Ok(res) => {
let outputs = ValidationOutputs {
let outputs = CandidateCommitments {
head_data: res.head_data,
upward_messages: res.upward_messages,
horizontal_messages: res.horizontal_messages,
......
......@@ -276,7 +276,7 @@ mod tests {
fn check_validation_outputs(
&self,
para_id: ParaId,
_commitments: polkadot_primitives::v1::ValidationOutputs,
_commitments: polkadot_primitives::v1::CandidateCommitments,
) -> bool {
self.validation_outputs_results
.get(&para_id)
......@@ -498,7 +498,7 @@ mod tests {
let relay_parent = [1; 32].into();
let para_a = 5.into();
let para_b = 6.into();
let commitments = polkadot_primitives::v1::ValidationOutputs::default();
let commitments = polkadot_primitives::v1::CandidateCommitments::default();
runtime_api.validation_outputs_results.insert(para_a, false);
runtime_api.validation_outputs_results.insert(para_b, true);
......
......@@ -627,7 +627,7 @@ where
};
// check the merkle proof
let root = &live_candidate.commitments.erasure_root;
let root = &live_candidate.descriptor.erasure_root;
let anticipated_hash = if let Ok(hash) = branch_hash(
root,
&message.erasure_chunk.proof,
......
......@@ -290,11 +290,11 @@ impl TestCandidateBuilder {
para_id: self.para_id,
pov_hash: self.pov_hash,
relay_parent: self.relay_parent,
erasure_root: self.erasure_root,
..Default::default()
},
commitments: CandidateCommitments {
head_data: self.head_data,
erasure_root: self.erasure_root,
..Default::default()
},
}
......@@ -323,7 +323,7 @@ fn helper_integrity() {
let message =
make_valid_availability_gossip(&test_state, candidate.hash(), 2, pov_block.clone());
let root = dbg!(&candidate.commitments.erasure_root);
let root = dbg!(&candidate.descriptor.erasure_root);
let anticipated_hash = branch_hash(
root,
......
......@@ -28,7 +28,7 @@ use polkadot_primitives::v1::{
Hash, CommittedCandidateReceipt, CandidateReceipt, CompactStatement,
EncodeAs, Signed, SigningContext, ValidatorIndex, ValidatorId,
UpwardMessage, ValidationCode, PersistedValidationData, ValidationData,
HeadData, PoV, CollatorPair, Id as ParaId, OutboundHrmpMessage, ValidationOutputs, CandidateHash,
HeadData, PoV, CollatorPair, Id as ParaId, OutboundHrmpMessage, CandidateCommitments, CandidateHash,
};
use polkadot_statement_table::{
generic::{
......@@ -144,7 +144,7 @@ pub enum InvalidCandidate {
pub enum ValidationResult {
/// Candidate is valid. The validation process yields these outputs and the persisted validation
/// data used to form inputs.
Valid(ValidationOutputs, PersistedValidationData),
Valid(CandidateCommitments, PersistedValidationData),
/// Candidate is invalid.
Invalid(InvalidCandidate),
}
......
......@@ -406,7 +406,7 @@ pub enum RuntimeApiRequest {
/// Sends back `true` if the validation outputs pass all acceptance criteria checks.
CheckValidationOutputs(
ParaId,
polkadot_primitives::v1::ValidationOutputs,
polkadot_primitives::v1::CandidateCommitments,
RuntimeApiSender<bool>,
),
/// Get the session index that a child of the block will have.
......
......@@ -142,6 +142,8 @@ pub struct CandidateDescriptor<H = Hash> {
pub persisted_validation_data_hash: Hash,
/// The blake2-256 hash of the pov.
pub pov_hash: Hash,
/// The root of a block's erasure encoding Merkle tree.
pub erasure_root: Hash,
/// Signature on blake2-256 of components of this receipt:
/// The parachain index, the relay parent, the validation data hash, and the pov_hash.
pub signature: CollatorSignature,
......@@ -341,24 +343,6 @@ pub struct TransientValidationData<N = BlockNumber> {
pub dmq_length: u32,
}
/// Outputs of validating a candidate.
#[derive(Encode, Decode)]
#[cfg_attr(feature = "std", derive(Clone, Debug, Default))]
pub struct ValidationOutputs {
/// The head-data produced by validation.
pub head_data: HeadData,
/// Upward messages to the relay chain.
pub upward_messages: Vec<UpwardMessage>,
/// The horizontal messages sent by the parachain.
pub horizontal_messages: Vec<OutboundHrmpMessage<Id>>,
/// The new validation code submitted by the execution, if any.
pub new_validation_code: Option<ValidationCode>,
/// The number of messages processed from the DMQ.
pub processed_downward_messages: u32,
/// The mark which specifies the block number up to which all inbound HRMP messages are processed.
pub hrmp_watermark: BlockNumber,
}
/// Commitments made in a `CandidateReceipt`. Many of these are outputs of validation.
#[derive(PartialEq, Eq, Clone, Encode, Decode)]
#[cfg_attr(feature = "std", derive(Debug, Default, Hash))]
......@@ -367,8 +351,6 @@ pub struct CandidateCommitments<N = BlockNumber> {
pub upward_messages: Vec<UpwardMessage>,
/// Horizontal messages sent by the parachain.
pub horizontal_messages: Vec<OutboundHrmpMessage<Id>>,
/// The root of a block's erasure encoding Merkle tree.
pub erasure_root: Hash,
/// New validation code.
pub new_validation_code: Option<ValidationCode>,
/// The head-data produced as a result of execution.
......@@ -761,7 +743,7 @@ sp_api::decl_runtime_apis! {
-> Option<PersistedValidationData<N>>;
/// Checks if the given validation outputs pass the acceptance criteria.
fn check_validation_outputs(para_id: Id, outputs: ValidationOutputs) -> bool;
fn check_validation_outputs(para_id: Id, outputs: CandidateCommitments) -> bool;
/// Returns the session index expected at a child of the block.
///
......
......@@ -80,6 +80,8 @@ struct CandidateDescriptor {
persisted_validation_data_hash: Hash,
/// The blake2-256 hash of the pov-block.
pov_hash: Hash,
/// The root of a block's erasure encoding Merkle tree.
erasure_root: Hash,
/// Signature on blake2-256 of components of this receipt:
/// The parachain index, the relay parent, the validation data hash, and the pov_hash.
signature: CollatorSignature,
......@@ -251,8 +253,6 @@ struct CandidateCommitments {
horizontal_messages: Vec<OutboundHrmpMessage>,
/// Messages destined to be interpreted by the Relay chain itself.
upward_messages: Vec<UpwardMessage>,
/// The root of a block's erasure encoding Merkle tree.
erasure_root: Hash,
/// New validation code.
new_validation_code: Option<ValidationCode>,
/// The head-data produced as a result of execution.
......@@ -275,27 +275,4 @@ struct SigningContext {
/// The session index this signature is in the context of.
session_index: SessionIndex,
}
```
## Validation Outputs
This struct encapsulates the outputs of candidate validation.
```rust
struct ValidationOutputs {
/// The head-data produced by validation.
head_data: HeadData,
/// The validation data, persisted.
validation_data: PersistedValidationData,
/// Messages directed to other paras routed via the relay chain.
horizontal_messages: Vec<OutboundHrmpMessage>,
/// Upwards messages to the relay chain.
upwards_messages: Vec<UpwardsMessage>,
/// The new validation code submitted by the execution, if any.
new_validation_code: Option<ValidationCode>,
/// The number of messages processed from the DMQ.
processed_downward_messages: u32,
/// The mark which specifies the block number up to which all inbound HRMP messages are processed.
hrmp_watermark: BlockNumber,
}
```
```
\ No newline at end of file
......@@ -503,7 +503,7 @@ Various modules request that the [Candidate Validation subsystem](../node/utilit
enum ValidationResult {
/// Candidate is valid, and here are the outputs and the validation data used to form inputs.
/// In practice, this should be a shared type so that validation caching can be done.
Valid(ValidationOutputs, PersistedValidationData),
Valid(CandidateCommitments, PersistedValidationData),
/// Candidate is invalid.
Invalid,
}
......
......@@ -1083,7 +1083,7 @@ sp_api::impl_runtime_apis! {
}
fn check_validation_outputs(
_: Id,
_: primitives::v1::ValidationOutputs,
_: primitives::v1::CandidateCommitments,
) -> bool {
false
}
......
......@@ -562,7 +562,7 @@ impl<T: Trait> Module<T> {
/// Run the acceptance criteria checks on the given candidate commitments.
pub(crate) fn check_validation_outputs(
para_id: ParaId,
validation_outputs: primitives::v1::ValidationOutputs,
validation_outputs: primitives::v1::CandidateCommitments,
) -> bool {
if let Err(err) = CandidateCheckContext::<T>::new().check_validation_outputs(
para_id,
......
......@@ -221,7 +221,7 @@ pub fn persisted_validation_data<T: initializer::Trait>(
/// Implementation for the `check_validation_outputs` function of the runtime API.
pub fn check_validation_outputs<T: initializer::Trait>(
para_id: ParaId,
outputs: primitives::v1::ValidationOutputs,
outputs: primitives::v1::CandidateCommitments,
) -> bool {
<inclusion::Module<T>>::check_validation_outputs(para_id, outputs)
}
......
......@@ -1078,7 +1078,7 @@ sp_api::impl_runtime_apis! {
None
}
fn check_validation_outputs(_: Id, _: primitives::v1::ValidationOutputs) -> bool {
fn check_validation_outputs(_: Id, _: primitives::v1::CandidateCommitments) -> bool {
false
}
......
......@@ -657,7 +657,7 @@ sp_api::impl_runtime_apis! {
fn check_validation_outputs(
para_id: Id,
outputs: primitives::v1::ValidationOutputs,
outputs: primitives::v1::CandidateCommitments,
) -> bool {
runtime_api_impl::check_validation_outputs::<Runtime>(para_id, outputs)
}
......
......@@ -650,7 +650,7 @@ sp_api::impl_runtime_apis! {
fn check_validation_outputs(
para_id: ParaId,
outputs: primitives::v1::ValidationOutputs,
outputs: primitives::v1::CandidateCommitments,
) -> bool {
runtime_impl::check_validation_outputs::<Runtime>(para_id, outputs)
}
......
......@@ -831,7 +831,7 @@ sp_api::impl_runtime_apis! {
fn check_validation_outputs(
_: Id,
_: primitives::v1::ValidationOutputs
_: primitives::v1::CandidateCommitments
) -> bool {
false
}
......
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