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

Add dispute types and change InclusionInherent to ParasInherent (#2791)

* dispute types

* add Debug to dispute primitives in std and InherentData

* use ParachainsInherentData on node-side

* change inclusion_inherent to paras_inherent

* RuntimeDebug

* add type parameter to PersistedValidationData users

* fix test client

* spaces

* fix collation-generation test

* fix provisioner tests

* remove references to inclusion inherent
parent 05cd0d24
Pipeline #132330 failed with stages
in 25 minutes and 38 seconds
......@@ -731,7 +731,7 @@ mod tests {
// correct descriptor
let expect_pov_hash = test_collation().proof_of_validity.hash();
let expect_validation_data_hash
= PersistedValidationData::<BlockNumber>::default().hash();
= PersistedValidationData::<Hash, BlockNumber>::default().hash();
let expect_relay_parent = Hash::repeat_byte(4);
let expect_payload = collator_signature_payload(
&expect_relay_parent,
......
......@@ -26,7 +26,7 @@ use polkadot_node_subsystem::{
};
use polkadot_overseer::OverseerHandler;
use polkadot_primitives::v1::{
Block, Hash, Header,
Block, Hash, Header, InherentData as ParachainsInherentData,
};
use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider};
use sc_telemetry::TelemetryHandle;
......@@ -206,24 +206,29 @@ where
let span = jaeger::Span::new(self.parent_header_hash, "propose");
let _span = span.child("get-provisioner");
let provisioner_data = match self.get_provisioner_data().await {
Ok(pd) => pd,
let parachains_inherent_data = match self.get_provisioner_data().await {
Ok(pd) => ParachainsInherentData {
bitfields: pd.bitfields,
backed_candidates: pd.backed_candidates,
disputes: pd.disputes,
parent_header: self.parent_header,
},
Err(err) => {
tracing::warn!(err = ?err, "could not get provisioner inherent data; injecting default data");
Default::default()
ParachainsInherentData {
bitfields: Vec::new(),
backed_candidates: Vec::new(),
disputes: Vec::new(),
parent_header: self.parent_header,
}
}
};
drop(_span);
let inclusion_inherent_data = (
provisioner_data.0,
provisioner_data.1,
self.parent_header,
);
inherent_data.put_data(
polkadot_primitives::v1::INCLUSION_INHERENT_IDENTIFIER,
&inclusion_inherent_data,
polkadot_primitives::v1::PARACHAINS_INHERENT_IDENTIFIER,
&parachains_inherent_data,
)?;
let _span = span.child("authorship-propose");
......
......@@ -296,12 +296,16 @@ async fn send_inherent_data(
candidates,
relay_parent,
from_job,
)
.await?;
).await?;
let inherent_data = ProvisionerInherentData {
bitfields,
backed_candidates: candidates,
disputes: Vec::new(), // until disputes are implemented.
};
let res = (bitfields, candidates);
for return_sender in return_senders {
return_sender.send(res.clone()).map_err(|_data| Error::InherentDataReturnChannel)?;
return_sender.send(inherent_data.clone()).map_err(|_data| Error::InherentDataReturnChannel)?;
}
Ok(())
......
......@@ -342,7 +342,7 @@ mod select_candidates {
let mock_cores = mock_availability_cores();
let n_cores = mock_cores.len();
let empty_hash = PersistedValidationData::<BlockNumber>::default().hash();
let empty_hash = PersistedValidationData::<Hash, BlockNumber>::default().hash();
let candidate_template = CandidateReceipt {
descriptor: CandidateDescriptor {
......@@ -415,7 +415,7 @@ mod select_candidates {
let mock_cores = mock_availability_cores();
let n_cores = mock_cores.len();
let empty_hash = PersistedValidationData::<BlockNumber>::default().hash();
let empty_hash = PersistedValidationData::<Hash, BlockNumber>::default().hash();
// why those particular indices? see the comments on mock_availability_cores()
// the first candidate with code is included out of [1, 4, 7, 8, 10].
......
......@@ -44,7 +44,7 @@ use polkadot_primitives::v1::{
PersistedValidationData, SessionIndex, SignedAvailabilityBitfield,
ValidationCode, ValidatorId, CandidateHash,
ValidatorIndex, ValidatorSignature, InboundDownwardMessage, InboundHrmpMessage,
CandidateIndex, GroupIndex,
CandidateIndex, GroupIndex, MultiDisputeStatementSet, SignedAvailabilityBitfields,
};
use polkadot_statement_table::v1::Misbehavior;
use polkadot_procmacro_subsystem_dispatch_gen::subsystem_dispatch_gen;
......@@ -550,10 +550,16 @@ pub enum ProvisionableData {
Dispute(Hash, ValidatorSignature),
}
/// This data needs to make its way from the provisioner into the InherentData.
///
/// There, it is used to construct the ParaInherent.
pub type ProvisionerInherentData = (Vec<SignedAvailabilityBitfield>, Vec<BackedCandidate>);
/// Inherent data returned by the provisioner
#[derive(Debug, Clone)]
pub struct ProvisionerInherentData {
/// Signed bitfields.
pub bitfields: SignedAvailabilityBitfields,
/// Backed candidates.
pub backed_candidates: Vec<BackedCandidate>,
/// Dispute statement sets.
pub disputes: MultiDisputeStatementSet,
}
/// Message to the Provisioner.
///
......
......@@ -16,7 +16,7 @@
use crate::{Client, FullBackend};
use polkadot_test_runtime::{GetLastTimestamp, UncheckedExtrinsic};
use polkadot_primitives::v1::Block;
use polkadot_primitives::v1::{Block, InherentData as ParachainsInherentData};
use sp_runtime::{generic::BlockId, Digest, DigestItem};
use sp_api::ProvideRuntimeApi;
use sp_consensus_babe::{BABE_ENGINE_ID, digests::{PreDigest, SecondaryPlainPreDigest}};
......@@ -100,18 +100,20 @@ impl InitPolkadotBlockBuilder for Client {
let parent_header = self.header(at)
.expect("Get the parent block header")
.expect("The target block header must exist");
let provisioner_data = polkadot_node_subsystem::messages::ProvisionerInherentData::default();
let inclusion_inherent_data = (
provisioner_data.0,
provisioner_data.1,
parent_header,
);
let parachains_inherent_data = ParachainsInherentData {
bitfields: Vec::new(),
backed_candidates: Vec::new(),
disputes: Vec::new(),
parent_header: parent_header,
};
inherent_data
.put_data(
polkadot_primitives::v1::INCLUSION_INHERENT_IDENTIFIER,
&inclusion_inherent_data,
polkadot_primitives::v1::PARACHAINS_INHERENT_IDENTIFIER,
&parachains_inherent_data,
)
.expect("Put inclusion inherent data");
.expect("Put parachains inherent data");
let inherents = block_builder.create_inherents(inherent_data).expect("Creates inherents");
......
......@@ -22,7 +22,7 @@ use parity_scale_codec::{Encode, Decode};
use bitvec::vec::BitVec;
use primitives::RuntimeDebug;
use runtime_primitives::traits::AppVerify;
use runtime_primitives::traits::{AppVerify, Header as HeaderT};
use inherents::InherentIdentifier;
use sp_arithmetic::traits::{BaseArithmetic, Saturating};
use application_crypto::KeyTypeId;
......@@ -169,8 +169,8 @@ pub mod well_known_keys {
}
}
/// Unique identifier for the Inclusion Inherent
pub const INCLUSION_INHERENT_IDENTIFIER: InherentIdentifier = *b"inclusn0";
/// Unique identifier for the Parachains Inherent
pub const PARACHAINS_INHERENT_IDENTIFIER: InherentIdentifier = *b"parachn0";
/// The key type ID for parachain assignment key.
pub const ASSIGNMENT_KEY_TYPE_ID: KeyTypeId = KeyTypeId(*b"asgn");
......@@ -316,7 +316,7 @@ pub struct FullCandidateReceipt<H = Hash, N = BlockNumber> {
/// point. The hash of the persisted validation data should
/// match the `persisted_validation_data_hash` in the descriptor
/// of the receipt.
pub validation_data: PersistedValidationData<N>,
pub validation_data: PersistedValidationData<H, N>,
}
/// A candidate-receipt with commitments directly included.
......@@ -395,18 +395,18 @@ impl Ord for CommittedCandidateReceipt {
/// during inclusion for each candidate and therefore lies on the critical path of inclusion.
#[derive(PartialEq, Eq, Clone, Encode, Decode)]
#[cfg_attr(feature = "std", derive(Debug, Default, MallocSizeOf))]
pub struct PersistedValidationData<N = BlockNumber> {
pub struct PersistedValidationData<H = Hash, N = BlockNumber> {
/// The parent head-data.
pub parent_head: HeadData,
/// The relay-chain block number this is in the context of.
pub relay_parent_number: N,
/// The relay-chain block storage root this is in the context of.
pub relay_parent_storage_root: Hash,
pub relay_parent_storage_root: H,
/// The maximum legal size of a POV block, in bytes.
pub max_pov_size: u32,
}
impl<N: Encode> PersistedValidationData<N> {
impl<H: Encode, N: Encode> PersistedValidationData<H, N> {
/// Compute the blake2-256 hash of the persisted validation data.
pub fn hash(&self) -> Hash {
BlakeTwo256::hash_of(self)
......@@ -820,7 +820,7 @@ sp_api::decl_runtime_apis! {
/// and the para already occupies a core.
#[skip_initialize_block]
fn persisted_validation_data(para_id: Id, assumption: OccupiedCoreAssumption)
-> Option<PersistedValidationData<N>>;
-> Option<PersistedValidationData<H, N>>;
/// Checks if the given validation outputs pass the acceptance criteria.
#[skip_initialize_block]
......@@ -991,6 +991,95 @@ impl<H> From<ConsensusLog> for runtime_primitives::DigestItem<H> {
}
}
/// A statement about a candidate, to be used within the dispute resolution process.
///
/// Statements are either in favor of the candidate's validity or against it.
#[derive(Encode, Decode, Clone, PartialEq, RuntimeDebug)]
pub enum DisputeStatement {
/// A valid statement, of the given kind.
#[codec(index = 0)]
Valid(ValidDisputeStatementKind),
/// An invalid statement, of the given kind.
#[codec(index = 1)]
Invalid(InvalidDisputeStatementKind),
}
/// Different kinds of statements of validity on a candidate.
#[derive(Encode, Decode, Clone, PartialEq, RuntimeDebug)]
pub enum ValidDisputeStatementKind {
/// An explicit statement issued as part of a dispute.
#[codec(index = 0)]
Explicit,
/// A seconded statement on a candidate from the backing phase.
#[codec(index = 1)]
BackingSeconded,
/// A valid statement on a candidate from the backing phase.
#[codec(index = 2)]
BackingValid,
/// An approval vote from the approval checking phase.
#[codec(index = 3)]
ApprovalChecking,
}
/// Different kinds of statements of invalidity on a candidate.
#[derive(Encode, Decode, Clone, PartialEq, RuntimeDebug)]
pub enum InvalidDisputeStatementKind {
/// An explicit statement issued as part of a dispute.
#[codec(index = 0)]
Explicit,
}
/// An explicit statement on a candidate issued as part of a dispute.
#[derive(Encode, Decode, Clone, PartialEq, RuntimeDebug)]
pub struct ExplicitDisputeStatement {
/// Whether the candidate is valid
pub valid: bool,
/// The candidate hash.
pub candidate_hash: CandidateHash,
/// The session index of the candidate.
pub session: SessionIndex,
}
/// A set of statements about a specific candidate.
#[derive(Encode, Decode, Clone, PartialEq, RuntimeDebug)]
pub struct DisputeStatementSet {
/// The candidate referenced by this set.
pub candidate_hash: CandidateHash,
/// The session index of the candidate.
pub session: SessionIndex,
/// Statements about the candidate.
pub statements: Vec<(DisputeStatement, ValidatorIndex, ValidatorSignature)>,
}
/// A set of dispute statements.
pub type MultiDisputeStatementSet = Vec<DisputeStatementSet>;
/// The entire state of a dispute.
#[derive(Encode, Decode, Clone, RuntimeDebug)]
pub struct DisputeState<N = BlockNumber> {
/// A bitfield indicating all validators for the candidate.
pub validators_for: BitVec<bitvec::order::Lsb0, u8>, // one bit per validator.
/// A bitfield indicating all validators against the candidate.
pub validators_against: BitVec<bitvec::order::Lsb0, u8>, // one bit per validator.
/// The block number at which the dispute started on-chain.
pub start: N,
/// The block number at which the dispute concluded on-chain.
pub concluded_at: Option<N>,
}
/// Parachains inherent-data passed into the runtime by a block author
#[derive(Encode, Decode, Clone, PartialEq, RuntimeDebug)]
pub struct InherentData<HDR: HeaderT = Header> {
/// Signed bitfields by validators about availability.
pub bitfields: SignedAvailabilityBitfields,
/// Backed candidates for inclusion in the block.
pub backed_candidates: Vec<BackedCandidate<HDR::Hash>>,
/// Sets of dispute votes for inclusion,
pub disputes: MultiDisputeStatementSet,
/// The parent block header. Used for checking state proofs.
pub parent_header: HDR,
}
#[cfg(test)]
mod tests {
use super::*;
......
......@@ -1173,7 +1173,7 @@ sp_api::impl_runtime_apis! {
}
fn persisted_validation_data(_: Id, _: OccupiedCoreAssumption)
-> Option<PersistedValidationData<BlockNumber>> {
-> Option<PersistedValidationData<Hash, BlockNumber>> {
None
}
fn check_validation_outputs(
......
......@@ -25,7 +25,7 @@ use primitives::v1::{
CandidateCommitments, CandidateDescriptor, ValidatorIndex, Id as ParaId,
AvailabilityBitfield as AvailabilityBitfield, SignedAvailabilityBitfields, SigningContext,
BackedCandidate, CoreIndex, GroupIndex, CommittedCandidateReceipt,
CandidateReceipt, HeadData, CandidateHash, Hash,
CandidateReceipt, HeadData, CandidateHash,
};
use frame_support::{
decl_storage, decl_module, decl_error, decl_event, ensure, dispatch::DispatchResult, IterableStorageMap,
......@@ -378,7 +378,7 @@ impl<T: Config> Module<T> {
/// Both should be sorted ascending by core index, and the candidates should be a subset of
/// scheduled cores. If these conditions are not met, the execution of the function fails.
pub(crate) fn process_candidates(
parent_storage_root: Hash,
parent_storage_root: T::Hash,
candidates: Vec<BackedCandidate<T::Hash>>,
scheduled: Vec<CoreAssignment>,
group_validators: impl Fn(GroupIndex) -> Option<Vec<ValidatorIndex>>,
......
......@@ -25,9 +25,9 @@
pub mod configuration;
pub mod shared;
pub mod inclusion;
pub mod inclusion_inherent;
pub mod initializer;
pub mod paras;
pub mod paras_inherent;
pub mod scheduler;
pub mod session_info;
pub mod origin;
......
......@@ -134,7 +134,7 @@ impl crate::inclusion::Config for Test {
type RewardValidators = TestRewardValidators;
}
impl crate::inclusion_inherent::Config for Test { }
impl crate::paras_inherent::Config for Test { }
impl crate::session_info::Config for Test { }
......
......@@ -22,8 +22,9 @@
//! this module.
use sp_std::prelude::*;
use sp_runtime::traits::Header as HeaderT;
use primitives::v1::{
BackedCandidate, SignedAvailabilityBitfields, INCLUSION_INHERENT_IDENTIFIER, Header,
BackedCandidate, PARACHAINS_INHERENT_IDENTIFIER, InherentData as ParachainsInherentData,
};
use frame_support::{
decl_error, decl_module, decl_storage, ensure,
......@@ -43,14 +44,14 @@ const LOG_TARGET: &str = "runtime::inclusion-inherent";
// In the future, we should benchmark these consts; these are all untested assumptions for now.
const BACKED_CANDIDATE_WEIGHT: Weight = 100_000;
const INCLUSION_INHERENT_CLAIMED_WEIGHT: Weight = 1_000_000_000;
// we assume that 75% of an inclusion inherent's weight is used processing backed candidates
// we assume that 75% of an paras inherent's weight is used processing backed candidates
const MINIMAL_INCLUSION_INHERENT_WEIGHT: Weight = INCLUSION_INHERENT_CLAIMED_WEIGHT / 4;
pub trait Config: inclusion::Config + scheduler::Config {}
decl_storage! {
trait Store for Module<T: Config> as ParaInclusionInherent {
/// Whether the inclusion inherent was included within this block.
trait Store for Module<T: Config> as ParaInherent {
/// Whether the paras inherent was included within this block.
///
/// The `Option<()>` is effectively a bool, but it never hits storage in the `None` variant
/// due to the guarantees of FRAME's storage APIs.
......@@ -71,7 +72,7 @@ decl_error! {
}
decl_module! {
/// The inclusion inherent module.
/// The paras inherent module.
pub struct Module<T: Config> for enum Call where origin: <T as frame_system::Config>::Origin {
type Error = Error<T>;
......@@ -85,17 +86,22 @@ decl_module! {
}
}
/// Include backed candidates and bitfields.
/// Enter the paras inherent. This will process bitfields and backed candidates.
#[weight = (
MINIMAL_INCLUSION_INHERENT_WEIGHT + backed_candidates.len() as Weight * BACKED_CANDIDATE_WEIGHT,
MINIMAL_INCLUSION_INHERENT_WEIGHT + data.backed_candidates.len() as Weight * BACKED_CANDIDATE_WEIGHT,
DispatchClass::Mandatory,
)]
pub fn inclusion(
pub fn enter(
origin,
signed_bitfields: SignedAvailabilityBitfields,
backed_candidates: Vec<BackedCandidate<T::Hash>>,
parent_header: Header,
data: ParachainsInherentData<T::Header>,
) -> DispatchResultWithPostInfo {
let ParachainsInherentData {
bitfields: signed_bitfields,
backed_candidates,
parent_header,
disputes: _,
} = data;
ensure_none(origin)?;
ensure!(!<Included>::exists(), Error::<T>::TooManyInclusionInherents);
......@@ -137,7 +143,7 @@ decl_module! {
let backed_candidates_len = backed_candidates.len() as Weight;
// Process backed candidates according to scheduled cores.
let parent_storage_root = parent_header.state_root;
let parent_storage_root = parent_header.state_root().clone();
let occupied = <inclusion::Module<T>>::process_candidates(
parent_storage_root,
backed_candidates,
......@@ -195,7 +201,7 @@ fn limit_backed_candidates<T: Config>(
});
}
// the weight of the inclusion inherent is already included in the current block weight,
// the weight of the paras inherent is already included in the current block weight,
// so our operation is simple: if the block is currently overloaded, make this intrinsic smaller
if frame_system::Pallet::<T>::block_weight().total() > <T as frame_system::Config>::BlockWeights::get().max_block {
Vec::new()
......@@ -207,39 +213,49 @@ fn limit_backed_candidates<T: Config>(
impl<T: Config> ProvideInherent for Module<T> {
type Call = Call<T>;
type Error = MakeFatalError<()>;
const INHERENT_IDENTIFIER: InherentIdentifier = INCLUSION_INHERENT_IDENTIFIER;
const INHERENT_IDENTIFIER: InherentIdentifier = PARACHAINS_INHERENT_IDENTIFIER;
fn create_inherent(data: &InherentData) -> Option<Self::Call> {
data.get_data(&Self::INHERENT_IDENTIFIER)
.expect("inclusion inherent data failed to decode")
.map(
|(signed_bitfields, backed_candidates, parent_header): (
SignedAvailabilityBitfields,
Vec<BackedCandidate<T::Hash>>,
Header,
)| {
// Sanity check: session changes can invalidate an inherent, and we _really_ don't want that to happen.
// See github.com/paritytech/polkadot/issues/1327
let (signed_bitfields, backed_candidates) = match Self::inclusion(
frame_system::RawOrigin::None.into(),
signed_bitfields.clone(),
backed_candidates.clone(),
parent_header.clone(),
) {
Ok(_) => (signed_bitfields, backed_candidates),
Err(err) => {
log::warn!(
target: LOG_TARGET,
"dropping signed_bitfields and backed_candidates because they produced \
an invalid inclusion inherent: {:?}",
err,
);
(Vec::new().into(), Vec::new())
}
};
Call::inclusion(signed_bitfields, backed_candidates, parent_header)
let inherent_data: ParachainsInherentData<T::Header>
= match data.get_data(&Self::INHERENT_IDENTIFIER)
{
Ok(Some(d)) => d,
Ok(None) => return None,
Err(_) => {
log::warn!(
target: LOG_TARGET,
"ParachainsInherentData failed to decode",
);
return None;
}
};
// Sanity check: session changes can invalidate an inherent, and we _really_ don't want that to happen.
// See github.com/paritytech/polkadot/issues/1327
let inherent_data = match Self::enter(
frame_system::RawOrigin::None.into(),
inherent_data.clone(),
) {
Ok(_) => inherent_data,
Err(err) => {
log::warn!(
target: LOG_TARGET,
"dropping signed_bitfields and backed_candidates because they produced \
an invalid paras inherent: {:?}",
err,
);
ParachainsInherentData {
bitfields: Vec::new(),
backed_candidates: Vec::new(),
disputes: Vec::new(),
parent_header: inherent_data.parent_header,
}
)
}
};
Some(Call::enter(inherent_data))
}
}
......@@ -307,12 +323,13 @@ mod tests {
}
}
mod inclusion_inherent_weight {
mod paras_inherent_weight {
use super::*;
use crate::mock::{
new_test_ext, System, MockGenesisConfig, Test
};
use primitives::v1::Header;
use frame_support::traits::UnfilteredDispatchable;
......@@ -326,7 +343,7 @@ mod tests {
}
}
/// We expect the weight of the inclusion inherent not to change when no truncation occurs:
/// We expect the weight of the paras inherent not to change when no truncation occurs:
/// its weight is dynamically computed from the size of the backed candidates list, and is
/// already incorporated into the current block weight when it is selected by the provisioner.
#[test]
......@@ -336,7 +353,7 @@ mod tests {
System::set_block_number(1);
System::set_parent_hash(header.hash());
// number of bitfields doesn't affect the inclusion inherent weight, so we can mock it with an empty one
// number of bitfields doesn't affect the paras inherent weight, so we can mock it with an empty one
let signed_bitfields = Vec::new();
// backed candidates must not be empty, so we can demonstrate that the weight has not changed
let backed_candidates = vec![BackedCandidate::default(); 10];
......@@ -350,8 +367,13 @@ mod tests {
let used_block_weight = max_block_weight / 2;
System::set_block_consumed_resources(used_block_weight, 0);
// execute the inclusion inherent
let post_info = Call::<Test>::inclusion(signed_bitfields, backed_candidates, default_header())
// execute the paras inherent
let post_info = Call::<Test>::enter(ParachainsInherentData {
bitfields: signed_bitfields,
backed_candidates,
disputes: Vec::new(),
parent_header: default_header(),
})
.dispatch_bypass_filter(None.into()).unwrap_err().post_info;
// we don't directly check the block's weight post-call. Instead, we check that the
......@@ -367,7 +389,7 @@ mod tests {
});
}
/// We expect the weight of the inclusion inherent to change when truncation occurs: its
/// We expect the weight of the paras inherent to change when truncation occurs: its
/// weight was initially dynamically computed from the size of the backed candidates list,
/// but was reduced by truncation.
#[test]
......@@ -377,7 +399,7 @@ mod tests {
System::set_block_number(1);
System::set_parent_hash(header.hash());
// number of bitfields doesn't affect the inclusion inherent weight, so we can mock it with an empty one
// number of bitfields doesn't affect the paras inherent weight, so we can mock it with an empty one
let signed_bitfields = Vec::new();
// backed candidates must not be empty, so we can demonstrate that the weight has not changed
let backed_candidates = vec![BackedCandidate::default(); 10];
......@@ -390,8 +412,13 @@ mod tests {
let used_block_weight = max_block_weight + 1;
System::set_block_consumed_resources(used_block_weight, 0);
// execute the inclusion inherent
let post_info = Call::<Test>::inclusion(signed_bitfields, backed_candidates, header)
// execute the paras inherent
let post_info = Call::<Test>::enter(ParachainsInherentData {