Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Use a BoundedVec in ValidationResult #6603

Merged
merged 10 commits into from
Feb 16, 2023
Merged

Use a BoundedVec in ValidationResult #6603

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0107d44
Use a `BoundedVec` in `ValidationResult`
mrcnski Jan 22, 2023
0e51e1b
List all fields when hashing so we don't miss one
mrcnski Jan 26, 2023
aaa548f
Define types for `BoundedVec`s of messages
mrcnski Jan 26, 2023
d83ec70
Fix test compile errors
mrcnski Jan 26, 2023
30784ca
Merge branch 'master' into mrcnski/boundedvec-validationresult
mrcnski Feb 2, 2023
7789176
Depend on `bounded-collections` 0.1.4 (fixes allocation issue)
mrcnski Feb 2, 2023
adbe1e4
Fix compilation issue
mrcnski Feb 2, 2023
102e309
Merge branch 'master' into mrcnski/boundedvec-validationresult
mrcnski Feb 16, 2023
ee1faa3
Derive `Hash` instead of manual `impl`
mrcnski Feb 16, 2023
0ff5be5
Avoid use of unwrap
mrcnski Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions node/collation-generation/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ mod handle_new_activations {

fn test_collation() -> Collation {
Collation {
upward_messages: vec![],
horizontal_messages: vec![],
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
new_validation_code: None,
head_data: dummy_head_data(),
proof_of_validity: MaybeCompressedPoV::Raw(PoV { block_data: BlockData(Vec::new()) }),
Expand Down
20 changes: 10 additions & 10 deletions node/core/backing/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ impl TestCandidateBuilder {
},
commitments: CandidateCommitments {
head_data: self.head_data,
upward_messages: vec![],
horizontal_messages: vec![],
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
new_validation_code: None,
processed_downward_messages: 0,
hrmp_watermark: 0_u32,
Expand Down Expand Up @@ -311,8 +311,8 @@ fn backing_second_works() {
tx.send(Ok(
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
horizontal_messages: Vec::new(),
upward_messages: Vec::new(),
horizontal_messages: vec![].try_into().unwrap(),
upward_messages: vec![].try_into().unwrap(),
new_validation_code: None,
processed_downward_messages: 0,
hrmp_watermark: 0,
Expand Down Expand Up @@ -457,8 +457,8 @@ fn backing_works() {
tx.send(Ok(
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
new_validation_code: None,
processed_downward_messages: 0,
hrmp_watermark: 0,
Expand Down Expand Up @@ -781,8 +781,8 @@ fn backing_misbehavior_works() {
tx.send(Ok(
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
new_validation_code: None,
processed_downward_messages: 0,
hrmp_watermark: 0,
Expand Down Expand Up @@ -954,8 +954,8 @@ fn backing_dont_second_invalid() {
tx.send(Ok(
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
new_validation_code: None,
processed_downward_messages: 0,
hrmp_watermark: 0,
Expand Down
24 changes: 12 additions & 12 deletions node/core/candidate-validation/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,8 @@ fn candidate_validation_ok_is_ok() {
let validation_result = WasmValidationResult {
head_data,
new_validation_code: Some(vec![2, 2, 2].into()),
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
processed_downward_messages: 0,
hrmp_watermark: 0,
};
Expand Down Expand Up @@ -573,8 +573,8 @@ fn candidate_validation_one_ambiguous_error_is_valid() {
let validation_result = WasmValidationResult {
head_data,
new_validation_code: Some(vec![2, 2, 2].into()),
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
processed_downward_messages: 0,
hrmp_watermark: 0,
};
Expand Down Expand Up @@ -751,8 +751,8 @@ fn candidate_validation_commitment_hash_mismatch_is_invalid() {
let validation_result = WasmValidationResult {
head_data,
new_validation_code: None,
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
processed_downward_messages: 0,
hrmp_watermark: 12345,
};
Expand Down Expand Up @@ -854,8 +854,8 @@ fn compressed_code_works() {
let validation_result = WasmValidationResult {
head_data,
new_validation_code: None,
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
processed_downward_messages: 0,
hrmp_watermark: 0,
};
Expand Down Expand Up @@ -918,8 +918,8 @@ fn code_decompression_failure_is_error() {
let validation_result = WasmValidationResult {
head_data,
new_validation_code: None,
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
processed_downward_messages: 0,
hrmp_watermark: 0,
};
Expand Down Expand Up @@ -971,8 +971,8 @@ fn pov_decompression_failure_is_invalid() {
let validation_result = WasmValidationResult {
head_data,
new_validation_code: None,
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
processed_downward_messages: 0,
hrmp_watermark: 0,
};
Expand Down
4 changes: 2 additions & 2 deletions node/malus/src/variants/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ pub fn create_fake_candidate_commitments(
persisted_validation_data: &PersistedValidationData,
) -> CandidateCommitments {
CandidateCommitments {
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: Vec::new().try_into().unwrap(),
horizontal_messages: Vec::new().try_into().unwrap(),
new_validation_code: None,
head_data: persisted_validation_data.parent_head.clone(),
processed_downward_messages: 0,
Expand Down
1 change: 1 addition & 0 deletions node/primitives/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ sp-consensus-vrf = { git = "https://github.com/paritytech/substrate", branch = "
sp-consensus-babe = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-keystore = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-maybe-compressed-blob = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master" }
polkadot-parachain = { path = "../../parachain", default-features = false }
schnorrkel = "0.9.1"
thiserror = "1.0.31"
Expand Down
10 changes: 5 additions & 5 deletions node/primitives/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
use polkadot_primitives::{
BlakeTwo256, BlockNumber, CandidateCommitments, CandidateHash, CollatorPair,
CommittedCandidateReceipt, CompactStatement, EncodeAs, Hash, HashT, HeadData, Id as ParaId,
OutboundHrmpMessage, PersistedValidationData, SessionIndex, Signed, UncheckedSigned,
UpwardMessage, ValidationCode, ValidatorIndex, MAX_CODE_SIZE, MAX_POV_SIZE,
PersistedValidationData, SessionIndex, Signed, UncheckedSigned, ValidationCode, ValidatorIndex,
MAX_CODE_SIZE, MAX_POV_SIZE,
};
pub use sp_consensus_babe::{
AllowedSlots as BabeAllowedSlots, BabeEpochConfiguration, Epoch as BabeEpoch,
};

pub use polkadot_parachain::primitives::BlockData;
pub use polkadot_parachain::primitives::{BlockData, HorizontalMessages, UpwardMessages};

pub mod approval;

Expand Down Expand Up @@ -312,9 +312,9 @@ impl MaybeCompressedPoV {
#[cfg(not(target_os = "unknown"))]
pub struct Collation<BlockNumber = polkadot_primitives::BlockNumber> {
/// Messages destined to be interpreted by the Relay chain itself.
pub upward_messages: Vec<UpwardMessage>,
pub upward_messages: UpwardMessages,
/// The horizontal messages sent by the parachain.
pub horizontal_messages: Vec<OutboundHrmpMessage<ParaId>>,
pub horizontal_messages: HorizontalMessages,
/// New validation code.
pub new_validation_code: Option<ValidationCode>,
/// The head-data produced as a result of execution.
Expand Down
2 changes: 2 additions & 0 deletions parachain/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ sp-core = { git = "https://github.com/paritytech/substrate", branch = "master",
frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
polkadot-core-primitives = { path = "../core-primitives", default-features = false }
derive_more = "0.99.11"
bounded-collections = { version = "0.1.5", default-features = false }

# all optional crates.
serde = { version = "1.0.137", default-features = false, features = [ "derive" ], optional = true }
Expand All @@ -25,6 +26,7 @@ serde = { version = "1.0.137", default-features = false, features = [ "derive" ]
default = ["std"]
wasm-api = []
std = [
"bounded-collections/std",
"parity-scale-codec/std",
"scale-info/std",
"serde/std",
Expand Down
21 changes: 19 additions & 2 deletions parachain/src/primitives.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

use sp_std::vec::Vec;

use bounded_collections::{BoundedVec, ConstU32};
use frame_support::weights::Weight;
use parity_scale_codec::{CompactAs, Decode, Encode, MaxEncodedLen};
use scale_info::TypeInfo;
Expand Down Expand Up @@ -361,6 +362,22 @@ pub struct ValidationParams {
pub relay_parent_storage_root: Hash,
}

/// Maximum number of HRMP messages allowed per candidate.
///
/// We also use this as a generous limit, which still prevents possible memory exhaustion, from
/// malicious parachains that may otherwise return a huge amount of messages in `ValidationResult`.
pub const MAX_HORIZONTAL_MESSAGE_NUM: u32 = 16 * 1024;
/// Maximum number of UMP messages allowed per candidate.
///
/// We also use this as a generous limit, which still prevents possible memory exhaustion, from
/// malicious parachains that may otherwise return a huge amount of messages in `ValidationResult`.
pub const MAX_UPWARD_MESSAGE_NUM: u32 = 16 * 1024;

pub type UpwardMessages = BoundedVec<UpwardMessage, ConstU32<MAX_UPWARD_MESSAGE_NUM>>;

pub type HorizontalMessages =
BoundedVec<OutboundHrmpMessage<Id>, ConstU32<MAX_HORIZONTAL_MESSAGE_NUM>>;

/// The result of parachain validation.
// TODO: balance uploads (https://github.com/paritytech/polkadot/issues/220)
#[derive(PartialEq, Eq, Clone, Encode)]
Expand All @@ -371,9 +388,9 @@ pub struct ValidationResult {
/// An update to the validation code that should be scheduled in the relay chain.
pub new_validation_code: Option<ValidationCode>,
/// Upward messages send by the Parachain.
pub upward_messages: Vec<UpwardMessage>,
pub upward_messages: UpwardMessages,
/// Outbound horizontal messages sent by the parachain.
pub horizontal_messages: Vec<OutboundHrmpMessage<Id>>,
pub horizontal_messages: HorizontalMessages,
/// Number of downward messages that were processed by the Parachain.
///
/// It is expected that the Parachain processes them from first to last.
Expand Down
4 changes: 2 additions & 2 deletions parachain/test-parachains/adder/collator/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ impl Collator {
let pov = PoV { block_data: block_data.encode().into() };

let collation = Collation {
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: Vec::new().try_into().unwrap(),
horizontal_messages: Vec::new().try_into().unwrap(),
new_validation_code: None,
head_data: head_data.encode().into(),
proof_of_validity: MaybeCompressedPoV::Raw(pov.clone()),
Expand Down
6 changes: 4 additions & 2 deletions parachain/test-parachains/adder/src/wasm_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ pub extern "C" fn validate_block(params: *const u8, len: usize) -> u64 {
parachain::write_result(&ValidationResult {
head_data: GenericHeadData(new_head.encode()),
new_validation_code: None,
upward_messages: sp_std::vec::Vec::new(),
horizontal_messages: sp_std::vec::Vec::new(),
upward_messages: sp_std::vec::Vec::new().try_into().expect("empty vec fits into bounds"),
horizontal_messages: sp_std::vec::Vec::new()
.try_into()
.expect("empty vec fits into bounds"),
processed_downward_messages: 0,
hrmp_watermark: params.relay_parent_number,
})
Expand Down
4 changes: 2 additions & 2 deletions parachain/test-parachains/undying/collator/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ impl Collator {
let pov = PoV { block_data: block_data.encode().into() };

let collation = Collation {
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
new_validation_code: None,
head_data: head_data.encode().into(),
proof_of_validity: MaybeCompressedPoV::Raw(pov.clone()),
Expand Down
6 changes: 4 additions & 2 deletions parachain/test-parachains/undying/src/wasm_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ pub extern "C" fn validate_block(params: *const u8, len: usize) -> u64 {
parachain::write_result(&ValidationResult {
head_data: GenericHeadData(new_head.encode()),
new_validation_code: None,
upward_messages: sp_std::vec::Vec::new(),
horizontal_messages: sp_std::vec::Vec::new(),
upward_messages: sp_std::vec::Vec::new().try_into().expect("empty vec fits within bounds"),
horizontal_messages: sp_std::vec::Vec::new()
.try_into()
.expect("empty vec fits within bounds"),
processed_downward_messages: 0,
hrmp_watermark: params.relay_parent_number,
})
Expand Down
10 changes: 5 additions & 5 deletions primitives/src/v2/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ pub use polkadot_core_primitives::v2::{

// Export some polkadot-parachain primitives
pub use polkadot_parachain::primitives::{
HeadData, HrmpChannelId, Id, UpwardMessage, ValidationCode, ValidationCodeHash,
LOWEST_PUBLIC_ID, LOWEST_USER_ID,
HeadData, HorizontalMessages, HrmpChannelId, Id, UpwardMessage, UpwardMessages, ValidationCode,
ValidationCodeHash, LOWEST_PUBLIC_ID, LOWEST_USER_ID,
};

#[cfg(feature = "std")]
Expand Down Expand Up @@ -593,12 +593,12 @@ impl<H: Encode, N: Encode> PersistedValidationData<H, N> {

/// Commitments made in a `CandidateReceipt`. Many of these are outputs of validation.
#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo, RuntimeDebug)]
#[cfg_attr(feature = "std", derive(Hash, Default))]
#[cfg_attr(feature = "std", derive(Default, Hash))]
pub struct CandidateCommitments<N = BlockNumber> {
/// Messages destined to be interpreted by the Relay chain itself.
pub upward_messages: Vec<UpwardMessage>,
pub upward_messages: UpwardMessages,
/// Horizontal messages sent by the parachain.
pub horizontal_messages: Vec<OutboundHrmpMessage<Id>>,
pub horizontal_messages: HorizontalMessages,
/// New validation code.
pub new_validation_code: Option<ValidationCode>,
/// The head-data produced as a result of execution.
Expand Down
4 changes: 2 additions & 2 deletions primitives/test-helpers/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ pub fn dummy_candidate_receipt_bad_sig(
pub fn dummy_candidate_commitments(head_data: impl Into<Option<HeadData>>) -> CandidateCommitments {
CandidateCommitments {
head_data: head_data.into().unwrap_or(dummy_head_data()),
upward_messages: vec![],
upward_messages: vec![].try_into().expect("empty vec fits within bounds"),
new_validation_code: None,
horizontal_messages: vec![],
horizontal_messages: vec![].try_into().expect("empty vec fits within bounds"),
processed_downward_messages: 0,
hrmp_watermark: 0_u32,
}
Expand Down
1 change: 1 addition & 0 deletions runtime/parachains/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ primitives = { package = "polkadot-primitives", path = "../../primitives", defau
rand = { version = "0.8.5", default-features = false }
rand_chacha = { version = "0.3.1", default-features = false }
static_assertions = { version = "1.1.0", optional = true }
polkadot-parachain = { path = "../../parachain", default-features = false }
polkadot-runtime-metrics = { path = "../metrics", default-features = false}

[dev-dependencies]
Expand Down
8 changes: 4 additions & 4 deletions runtime/parachains/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {
availability_votes,
);
let commitments = CandidateCommitments::<u32> {
upward_messages: vec![],
horizontal_messages: vec![],
upward_messages: vec![].try_into().unwrap(),
horizontal_messages: vec![].try_into().unwrap(),
new_validation_code: None,
head_data: Self::mock_head_data(),
processed_downward_messages: 0,
Expand Down Expand Up @@ -535,8 +535,8 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {
validation_code_hash,
},
commitments: CandidateCommitments::<u32> {
upward_messages: Vec::new(),
horizontal_messages: Vec::new(),
upward_messages: vec![].try_into().unwrap(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason why you're using try_into().unwrap() here instead of just Default::default() (which would eliminate the unwrap)?

horizontal_messages: vec![].try_into().unwrap(),
new_validation_code: includes_code_upgrade
.map(|v| ValidationCode(vec![42u8; v as usize])),
head_data,
Expand Down
Loading