From d4fdbf7db9487fcd29cf7e056bb39390cbd07144 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Tue, 2 Feb 2021 17:35:13 +0100 Subject: [PATCH] Clean up PersistedValidationData (#2353) * PVD: `block_number`->`relay_parent_number` * ValidationParams: `relay_chain_height`->`relay_parent_number` * Expose DMQ MQC hash as a well-known-key This way the relay storage merkle proofs will be able to obtain the DMQ MQC hash and we will be able to remove the it from the PersistedValidationData struct. * PersistedValidationData: Remove HRMP MQC heads * PersistedValidationData: Remove `dmq_mqc_head` * Expose the HRMP ingress channel index as a well-known-key This way a parachain (PVF and collator) can find all the parachains that have an outbound channel to the given one. That allows in turn to find all the inbound channels for the given para. Having access to that allows the parachain to get the same information as the hrmp_mqc_heads now provide. * Rename `relay_storage_root` to `relay_parent_storage_root` --- polkadot/node/core/av-store/src/tests.rs | 6 +-- polkadot/node/core/backing/src/lib.rs | 6 +-- .../node/core/candidate-validation/src/lib.rs | 6 +-- .../availability-distribution/src/tests.rs | 6 +-- .../availability-recovery/src/tests.rs | 6 +-- polkadot/parachain/src/primitives.rs | 13 +----- .../test-parachains/adder/collator/src/lib.rs | 8 ++-- .../adder/src/wasm_validation.rs | 2 +- .../test-parachains/tests/adder/mod.rs | 18 +++----- .../tests/wasm_executor/mod.rs | 18 +++----- polkadot/primitives/src/v1.rs | 46 ++++++++++++++----- .../implementers-guide/src/types/README.md | 15 ++---- .../implementers-guide/src/types/candidate.md | 9 +--- polkadot/runtime/parachains/src/dmp.rs | 24 +++++++++- polkadot/runtime/parachains/src/hrmp.rs | 15 +++++- .../parachains/src/runtime_api_impl/v1.rs | 4 +- polkadot/runtime/parachains/src/util.rs | 10 ++-- 17 files changed, 112 insertions(+), 100 deletions(-) diff --git a/polkadot/node/core/av-store/src/tests.rs b/polkadot/node/core/av-store/src/tests.rs index 74cbc0cb39..f7b3475ef9 100644 --- a/polkadot/node/core/av-store/src/tests.rs +++ b/polkadot/node/core/av-store/src/tests.rs @@ -103,11 +103,9 @@ impl Default for TestState { fn default() -> Self { let persisted_validation_data = PersistedValidationData { parent_head: HeadData(vec![7, 8, 9]), - block_number: 5, - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_number: 5, max_pov_size: 1024, - relay_storage_root: Default::default(), + relay_parent_storage_root: Default::default(), }; let pruning_config = PruningConfig { diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 27dec01e5f..5a8a783695 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -1274,11 +1274,9 @@ mod tests { let validation_data = PersistedValidationData { parent_head: HeadData(vec![7, 8, 9]), - block_number: Default::default(), - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_number: Default::default(), max_pov_size: 1024, - relay_storage_root: Default::default(), + relay_parent_storage_root: Default::default(), }; Self { diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs index 47405a208e..9ff4a7ed5a 100644 --- a/polkadot/node/core/candidate-validation/src/lib.rs +++ b/polkadot/node/core/candidate-validation/src/lib.rs @@ -438,10 +438,8 @@ fn validate_candidate_exhaustive( let params = ValidationParams { parent_head: persisted_validation_data.parent_head.clone(), block_data: pov.block_data.clone(), - relay_chain_height: persisted_validation_data.block_number, - relay_storage_root: persisted_validation_data.relay_storage_root, - dmq_mqc_head: persisted_validation_data.dmq_mqc_head, - hrmp_mqc_heads: persisted_validation_data.hrmp_mqc_heads.clone(), + relay_parent_number: persisted_validation_data.relay_parent_number, + relay_parent_storage_root: persisted_validation_data.relay_parent_storage_root, }; match B::validate(backend_arg, &validation_code, params, spawn) { diff --git a/polkadot/node/network/availability-distribution/src/tests.rs b/polkadot/node/network/availability-distribution/src/tests.rs index f1573dc62d..59ae6ebc2a 100644 --- a/polkadot/node/network/availability-distribution/src/tests.rs +++ b/polkadot/node/network/availability-distribution/src/tests.rs @@ -201,11 +201,9 @@ impl Default for TestState { let persisted_validation_data = PersistedValidationData { parent_head: HeadData(vec![7, 8, 9]), - block_number: Default::default(), - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_number: Default::default(), max_pov_size: 1024, - relay_storage_root: Default::default(), + relay_parent_storage_root: Default::default(), }; let pov_block_a = PoV { diff --git a/polkadot/node/network/availability-recovery/src/tests.rs b/polkadot/node/network/availability-recovery/src/tests.rs index 3833b12820..5e1cc1ce58 100644 --- a/polkadot/node/network/availability-recovery/src/tests.rs +++ b/polkadot/node/network/availability-recovery/src/tests.rs @@ -362,11 +362,9 @@ impl Default for TestState { let persisted_validation_data = PersistedValidationData { parent_head: HeadData(vec![7, 8, 9]), - block_number: Default::default(), - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_number: Default::default(), max_pov_size: 1024, - relay_storage_root: Default::default(), + relay_parent_storage_root: Default::default(), }; let pov = PoV { diff --git a/polkadot/parachain/src/primitives.rs b/polkadot/parachain/src/primitives.rs index 0304d59d61..0083723e94 100644 --- a/polkadot/parachain/src/primitives.rs +++ b/polkadot/parachain/src/primitives.rs @@ -267,18 +267,9 @@ pub struct ValidationParams { /// The collation body. pub block_data: BlockData, /// The current relay-chain block number. - pub relay_chain_height: RelayChainBlockNumber, + pub relay_parent_number: RelayChainBlockNumber, /// The relay-chain block's storage root. - pub relay_storage_root: Hash, - /// The MQC head for the DMQ. - /// - /// The DMQ MQC head will be used by the validation function to authorize the downward messages - /// passed by the collator. - pub dmq_mqc_head: Hash, - /// The list of MQC heads for the inbound HRMP channels paired with the sender para ids. This - /// vector is sorted ascending by the para id and doesn't contain multiple entries with the same - /// sender. - pub hrmp_mqc_heads: Vec<(Id, Hash)>, + pub relay_parent_storage_root: Hash, } /// The result of parachain validation. diff --git a/polkadot/parachain/test-parachains/adder/collator/src/lib.rs b/polkadot/parachain/test-parachains/adder/collator/src/lib.rs index 2d75597025..d8d8d3c1bb 100644 --- a/polkadot/parachain/test-parachains/adder/collator/src/lib.rs +++ b/polkadot/parachain/test-parachains/adder/collator/src/lib.rs @@ -168,7 +168,7 @@ impl Collator { block_data: block_data.encode().into(), }, processed_downward_messages: 0, - hrmp_watermark: validation_data.block_number, + hrmp_watermark: validation_data.relay_parent_number, }; async move { Some(collation) }.boxed() @@ -230,10 +230,8 @@ mod tests { ValidationParams { parent_head: parent_head.encode().into(), block_data: collation.proof_of_validity.block_data, - relay_chain_height: 1, - relay_storage_root: Default::default(), - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_number: 1, + relay_parent_storage_root: Default::default(), }, &IsolationStrategy::InProcess, sp_core::testing::TaskExecutor::new(), diff --git a/polkadot/parachain/test-parachains/adder/src/wasm_validation.rs b/polkadot/parachain/test-parachains/adder/src/wasm_validation.rs index f7e46cad39..4de5784adf 100644 --- a/polkadot/parachain/test-parachains/adder/src/wasm_validation.rs +++ b/polkadot/parachain/test-parachains/adder/src/wasm_validation.rs @@ -41,7 +41,7 @@ pub extern "C" fn validate_block(params: *const u8, len: usize) -> u64 { upward_messages: sp_std::vec::Vec::new(), horizontal_messages: sp_std::vec::Vec::new(), processed_downward_messages: 0, - hrmp_watermark: params.relay_chain_height, + hrmp_watermark: params.relay_parent_number, } ) } diff --git a/polkadot/parachain/test-parachains/tests/adder/mod.rs b/polkadot/parachain/test-parachains/tests/adder/mod.rs index 9726d618e8..2dc5812081 100644 --- a/polkadot/parachain/test-parachains/tests/adder/mod.rs +++ b/polkadot/parachain/test-parachains/tests/adder/mod.rs @@ -67,10 +67,8 @@ fn execute_good_on_parent(isolation_strategy: IsolationStrategy) { ValidationParams { parent_head: GenericHeadData(parent_head.encode()), block_data: GenericBlockData(block_data.encode()), - relay_chain_height: 1, - relay_storage_root: Default::default(), - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_number: 1, + relay_parent_storage_root: Default::default(), }, &isolation_strategy, sp_core::testing::TaskExecutor::new(), @@ -107,10 +105,8 @@ fn execute_good_chain_on_parent() { ValidationParams { parent_head: GenericHeadData(parent_head.encode()), block_data: GenericBlockData(block_data.encode()), - relay_chain_height: number as RelayChainBlockNumber + 1, - relay_storage_root: Default::default(), - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_number: number as RelayChainBlockNumber + 1, + relay_parent_storage_root: Default::default(), }, &isolation_strategy, sp_core::testing::TaskExecutor::new(), @@ -148,10 +144,8 @@ fn execute_bad_on_parent() { ValidationParams { parent_head: GenericHeadData(parent_head.encode()), block_data: GenericBlockData(block_data.encode()), - relay_chain_height: 1, - relay_storage_root: Default::default(), - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_number: 1, + relay_parent_storage_root: Default::default(), }, &isolation_strategy, sp_core::testing::TaskExecutor::new(), diff --git a/polkadot/parachain/test-parachains/tests/wasm_executor/mod.rs b/polkadot/parachain/test-parachains/tests/wasm_executor/mod.rs index ebfe83f3eb..a034ae5908 100644 --- a/polkadot/parachain/test-parachains/tests/wasm_executor/mod.rs +++ b/polkadot/parachain/test-parachains/tests/wasm_executor/mod.rs @@ -41,10 +41,8 @@ fn terminates_on_timeout() { ValidationParams { block_data: BlockData(Vec::new()), parent_head: Default::default(), - relay_chain_height: 1, - relay_storage_root: Default::default(), - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_number: 1, + relay_parent_storage_root: Default::default(), }, &isolation_strategy, sp_core::testing::TaskExecutor::new(), @@ -70,10 +68,8 @@ fn parallel_execution() { ValidationParams { block_data: BlockData(Vec::new()), parent_head: Default::default(), - relay_chain_height: 1, - relay_storage_root: Default::default(), - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_number: 1, + relay_parent_storage_root: Default::default(), }, &isolation_strategy, sp_core::testing::TaskExecutor::new(), @@ -83,10 +79,8 @@ fn parallel_execution() { ValidationParams { block_data: BlockData(Vec::new()), parent_head: Default::default(), - relay_storage_root: Default::default(), - relay_chain_height: 1, - hrmp_mqc_heads: Vec::new(), - dmq_mqc_head: Default::default(), + relay_parent_storage_root: Default::default(), + relay_parent_number: 1, }, &isolation_strategy_clone, sp_core::testing::TaskExecutor::new(), diff --git a/polkadot/primitives/src/v1.rs b/polkadot/primitives/src/v1.rs index 45fa88dec1..df2837310d 100644 --- a/polkadot/primitives/src/v1.rs +++ b/polkadot/primitives/src/v1.rs @@ -119,6 +119,22 @@ pub mod well_known_keys { }) } + /// The list of inbound channels for the given para. + /// + /// The storage entry stores a `Vec` + pub fn hrmp_ingress_channel_index(para_id: Id) -> Vec { + let prefix = hex!["6a0da05ca59913bc38a8630590f2627c1d3719f5b0b12c7105c073c507445948"]; + + para_id.using_encoded(|para_id: &[u8]| { + prefix.as_ref() + .iter() + .chain(twox_64(para_id).iter()) + .chain(para_id.iter()) + .cloned() + .collect() + }) + } + /// The list of outbound channels for the given para. /// /// The storage entry stores a `Vec` @@ -134,6 +150,23 @@ pub mod well_known_keys { .collect() }) } + + /// The MQC head for the downward message queue of the given para. See more in the `Dmp` module. + /// + /// The storage entry stores a `Hash`. This is polkadot hash which is at the moment + /// `blake2b-256`. + pub fn dmq_mqc_head(para_id: Id) -> Vec { + let prefix = hex!["63f78c98723ddc9073523ef3beefda0c4d7fefc408aac59dbfe80a72ac8e3ce5"]; + + para_id.using_encoded(|para_id: &[u8]| { + prefix.as_ref() + .iter() + .chain(twox_64(para_id).iter()) + .chain(para_id.iter()) + .cloned() + .collect() + }) + } } /// Unique identifier for the Inclusion Inherent @@ -360,18 +393,9 @@ pub struct PersistedValidationData { /// The parent head-data. pub parent_head: HeadData, /// The relay-chain block number this is in the context of. - pub block_number: N, + pub relay_parent_number: N, /// The relay-chain block storage root this is in the context of. - pub relay_storage_root: Hash, - /// The list of MQC heads for the inbound channels paired with the sender para ids. This - /// vector is sorted ascending by the para id and doesn't contain multiple entries with the same - /// sender. - pub hrmp_mqc_heads: Vec<(Id, Hash)>, - /// The MQC head for the DMQ. - /// - /// The DMQ MQC head will be used by the validation function to authorize the downward messages - /// passed by the collator. - pub dmq_mqc_head: Hash, + pub relay_parent_storage_root: Hash, /// The maximum legal size of a POV block, in bytes. pub max_pov_size: u32, } diff --git a/polkadot/roadmap/implementers-guide/src/types/README.md b/polkadot/roadmap/implementers-guide/src/types/README.md index ef68976084..7c8bec1535 100644 --- a/polkadot/roadmap/implementers-guide/src/types/README.md +++ b/polkadot/roadmap/implementers-guide/src/types/README.md @@ -93,17 +93,12 @@ digraph { PersistedValidationData<N = BlockNumber> parent_headHeadData block_numberN - relay_storage_rootHash - hrmp_mqc_headsVec<(Id, Hash)> - dmq_mqc_headHash + relay_parent_storage_rootHash max_pov_sizeu32 >] PersistedValidationData:parent_head -> HeadData:w - PersistedValidationData:hrmp_mqc_heads -> Id:w - PersistedValidationData:hrmp_mqc_heads -> MQCHash - PersistedValidationData:dmq_mqc_head -> MQCHash PersistedValidationDataHash [label = "Hash", shape="doublecircle", fill="gray90"] PersistedValidationDataHash -> PersistedValidationData:name @@ -401,16 +396,14 @@ digraph { ValidationParams parent_headHeadData block_dataBlockData - relay_chain_heightRelayChainBlockNumber - relay_storage_rootHash - dmq_mqc_headHash - hrmp_mqc_headsVec<(Id, Hash)> + relay_parent_numberRelayChainBlockNumber + relay_parent_storage_rootHash >] ValidationParams:parent_head -> HeadData:name ValidationParams:block_data -> BlockData:name - ValidationParams:relay_chain_height -> RelayChainBlockNumber:w + ValidationParams:relay_parent_number -> RelayChainBlockNumber:w RelayChainBlockNumber [label = "polkadot_core_primitives::BlockNumber"] diff --git a/polkadot/roadmap/implementers-guide/src/types/candidate.md b/polkadot/roadmap/implementers-guide/src/types/candidate.md index 0ba556a20d..7477c21c41 100644 --- a/polkadot/roadmap/implementers-guide/src/types/candidate.md +++ b/polkadot/roadmap/implementers-guide/src/types/candidate.md @@ -107,14 +107,9 @@ struct PersistedValidationData { /// The parent head-data. parent_head: HeadData, /// The relay-chain block number this is in the context of. This informs the collator. - block_number: BlockNumber, + relay_parent_number: BlockNumber, /// The relay-chain block storage root this is in the context of. - relay_storage_root: Hash, - /// The MQC head for the DMQ. - /// - /// The DMQ MQC head will be used by the validation function to authorize the downward messages - /// passed by the collator. - dmq_mqc_head: Hash, + relay_parent_storage_root: Hash, /// The list of MQC heads for the inbound channels paired with the sender para ids. This /// vector is sorted ascending by the para id and doesn't contain multiple entries with the same /// sender. diff --git a/polkadot/runtime/parachains/src/dmp.rs b/polkadot/runtime/parachains/src/dmp.rs index 4daa99ffb3..7930beeeb6 100644 --- a/polkadot/runtime/parachains/src/dmp.rs +++ b/polkadot/runtime/parachains/src/dmp.rs @@ -202,7 +202,8 @@ impl Module { /// Returns the Head of Message Queue Chain for the given para or `None` if there is none /// associated with it. - pub(crate) fn dmq_mqc_head(para: ParaId) -> Hash { + #[cfg(test)] + fn dmq_mqc_head(para: ParaId) -> Hash { ::DownwardMessageQueueHeads::get(¶) } @@ -406,4 +407,25 @@ mod tests { assert!(queue_downward_message(a, big).is_err()); }); } + + #[test] + fn verify_dmq_mqc_head_is_externally_accessible() { + use primitives::v1::well_known_keys; + use hex_literal::hex; + + let a = ParaId::from(2020); + + new_test_ext(default_genesis_config()).execute_with(|| { + let head = sp_io::storage::get(&well_known_keys::dmq_mqc_head(a)); + assert_eq!(head, None); + + queue_downward_message(a, vec![1, 2, 3]).unwrap(); + + let head = sp_io::storage::get(&well_known_keys::dmq_mqc_head(a)); + assert_eq!( + head, + Some(hex!["434f8579a2297dfea851bf6be33093c83a78b655a53ae141a7894494c0010589"].to_vec()) + ); + }); + } } diff --git a/polkadot/runtime/parachains/src/hrmp.rs b/polkadot/runtime/parachains/src/hrmp.rs index 55001e2801..c7db967dd5 100644 --- a/polkadot/runtime/parachains/src/hrmp.rs +++ b/polkadot/runtime/parachains/src/hrmp.rs @@ -1084,7 +1084,8 @@ impl Module { /// Returns the list of MQC heads for the inbound channels of the given recipient para paired /// with the sender para ids. This vector is sorted ascending by the para id and doesn't contain /// multiple entries with the same sender. - pub(crate) fn hrmp_mqc_heads(recipient: ParaId) -> Vec<(ParaId, Hash)> { + #[cfg(test)] + fn hrmp_mqc_heads(recipient: ParaId) -> Vec<(ParaId, Hash)> { let sender_set = ::HrmpIngressChannelsIndex::get(&recipient); // The ingress channels vector is sorted, thus `mqc_heads` is sorted as well. @@ -1706,6 +1707,18 @@ mod tests { }, ); + let raw_ingress_index = + sp_io::storage::get( + &well_known_keys::hrmp_ingress_channel_index(para_b), + ) + .expect("the ingress index must be present for para_b"); + let ingress_index = >::decode(&mut &raw_ingress_index[..]) + .expect("ingress indexx should be decodable as a list of para ids"); + assert_eq!( + ingress_index, + vec![para_a], + ); + // Now, verify that we can access and decode the egress index. let raw_egress_index = sp_io::storage::get( diff --git a/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs b/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs index f7fc8cceb3..7d77065df4 100644 --- a/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs +++ b/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs @@ -200,13 +200,13 @@ pub fn persisted_validation_data( ) -> Option> { use parity_scale_codec::Decode as _; let relay_parent_number = >::block_number(); - let relay_storage_root = Hash::decode(&mut &sp_io::storage::root()[..]) + let relay_parent_storage_root = Hash::decode(&mut &sp_io::storage::root()[..]) .expect("storage root must decode to the Hash type; qed"); with_assumption::(para_id, assumption, || { crate::util::make_persisted_validation_data::( para_id, relay_parent_number, - relay_storage_root, + relay_parent_storage_root, ) }) } diff --git a/polkadot/runtime/parachains/src/util.rs b/polkadot/runtime/parachains/src/util.rs index f2e4f6d6dd..f504b42aa1 100644 --- a/polkadot/runtime/parachains/src/util.rs +++ b/polkadot/runtime/parachains/src/util.rs @@ -19,7 +19,7 @@ use primitives::v1::{Id as ParaId, PersistedValidationData, Hash}; -use crate::{configuration, paras, dmp, hrmp}; +use crate::{configuration, paras, hrmp}; /// Make the persisted validation data for a particular parachain, a specified relay-parent and it's /// storage root. @@ -28,16 +28,14 @@ use crate::{configuration, paras, dmp, hrmp}; pub fn make_persisted_validation_data( para_id: ParaId, relay_parent_number: T::BlockNumber, - relay_storage_root: Hash, + relay_parent_storage_root: Hash, ) -> Option> { let config = >::config(); Some(PersistedValidationData { parent_head: >::para_head(¶_id)?, - block_number: relay_parent_number, - relay_storage_root, - hrmp_mqc_heads: >::hrmp_mqc_heads(para_id), - dmq_mqc_head: >::dmq_mqc_head(para_id), + relay_parent_number, + relay_parent_storage_root, max_pov_size: config.max_pov_size, }) }