From bbb2fbc5565c0d0824e4c7b533888f16d6626904 Mon Sep 17 00:00:00 2001 From: Ashley Date: Wed, 19 Feb 2020 17:33:38 +0100 Subject: [PATCH] Remove `TargetedMessage` (#848) * Remove TargetedMessage * Nitpicks --- .../network/src/legacy/tests/validation.rs | 2 +- polkadot/parachain/src/lib.rs | 32 +------ polkadot/parachain/src/wasm_api.rs | 10 +-- polkadot/parachain/src/wasm_executor/mod.rs | 5 +- .../src/wasm_executor/validation_host.rs | 31 +------ polkadot/parachain/tests/lib.rs | 5 +- polkadot/primitives/src/parachain.rs | 2 +- polkadot/validation/src/collation.rs | 87 ++----------------- polkadot/validation/src/lib.rs | 2 +- 9 files changed, 17 insertions(+), 159 deletions(-) diff --git a/polkadot/network/src/legacy/tests/validation.rs b/polkadot/network/src/legacy/tests/validation.rs index 24473d8661..a85e80faa6 100644 --- a/polkadot/network/src/legacy/tests/validation.rs +++ b/polkadot/network/src/legacy/tests/validation.rs @@ -28,7 +28,7 @@ use crate::legacy::{PolkadotProtocol, NetworkService, GossipService, GossipMessa use polkadot_validation::{SharedTable, Network}; use polkadot_primitives::{Block, BlockNumber, Hash, Header, BlockId}; use polkadot_primitives::parachain::{ - Id as ParaId, Chain, DutyRoster, ParachainHost, TargetedMessage, ValidatorId, Status, + Id as ParaId, Chain, DutyRoster, ParachainHost, ValidatorId, Status, FeeSchedule, HeadData, Retriable, CollatorId, ErasureChunk, CandidateReceipt, }; use parking_lot::Mutex; diff --git a/polkadot/parachain/src/lib.rs b/polkadot/parachain/src/lib.rs index 1cfd3b5c91..d1324140d7 100644 --- a/polkadot/parachain/src/lib.rs +++ b/polkadot/parachain/src/lib.rs @@ -48,7 +48,7 @@ pub mod wasm_executor; mod wasm_api; -use rstd::{vec::Vec, cmp::Ordering}; +use rstd::vec::Vec; use codec::{Encode, Decode, CompactAs}; use sp_core::{RuntimeDebug, TypeId}; @@ -224,33 +224,3 @@ pub struct IncomingMessage { /// The data of the message. pub data: Vec, } - -/// A message targeted to a specific parachain. -#[derive(Clone, PartialEq, Eq, Encode, Decode, sp_runtime_interface::pass_by::PassByCodec)] -#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize, Debug))] -#[cfg_attr(feature = "std", serde(rename_all = "camelCase"))] -#[cfg_attr(feature = "std", serde(deny_unknown_fields))] -pub struct TargetedMessage { - /// The target parachain. - pub target: Id, - /// The message data. - pub data: Vec, -} - -impl AsRef<[u8]> for TargetedMessage { - fn as_ref(&self) -> &[u8] { - &self.data[..] - } -} - -impl PartialOrd for TargetedMessage { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.target.cmp(&other.target)) - } -} - -impl Ord for TargetedMessage { - fn cmp(&self, other: &Self) -> Ordering { - self.target.cmp(&other.target) - } -} diff --git a/polkadot/parachain/src/wasm_api.rs b/polkadot/parachain/src/wasm_api.rs index 89cbcd6b37..ff47ddf8e2 100644 --- a/polkadot/parachain/src/wasm_api.rs +++ b/polkadot/parachain/src/wasm_api.rs @@ -16,7 +16,7 @@ //! Utilities for writing parachain WASM. -use crate::{TargetedMessage, UpwardMessage}; +use crate::UpwardMessage; use sp_runtime_interface::runtime_interface; #[cfg(feature = "std")] use sp_externalities::ExternalitiesExt; @@ -27,14 +27,6 @@ use sp_externalities::ExternalitiesExt; #[cfg(any(feature = "std", all(not(feature = "std"), feature = "wasm-api")))] #[runtime_interface] pub trait Parachain { - /// Post a message to another parachain. - fn post_message(&mut self, msg: TargetedMessage) { - self.extension::() - .expect("No `ParachainExt` associated with the current context.") - .post_message(msg) - .expect("Failed to post message") - } - /// Post a message to this parachain's relay chain. fn post_upward_message(&mut self, msg: UpwardMessage) { self.extension::() diff --git a/polkadot/parachain/src/wasm_executor/mod.rs b/polkadot/parachain/src/wasm_executor/mod.rs index bdf09507e7..1b6f556ac6 100644 --- a/polkadot/parachain/src/wasm_executor/mod.rs +++ b/polkadot/parachain/src/wasm_executor/mod.rs @@ -21,7 +21,7 @@ //! a WASM VM for re-execution of a parachain candidate. use std::any::{TypeId, Any}; -use crate::{ValidationParams, ValidationResult, UpwardMessage, TargetedMessage}; +use crate::{ValidationParams, ValidationResult, UpwardMessage}; use codec::{Decode, Encode}; use sp_core::storage::{ChildStorageKey, ChildInfo}; @@ -102,9 +102,6 @@ impl std::error::Error for Error { /// Externalities for parachain validation. pub trait Externalities: Send { - /// Called when a message is to be posted to another parachain. - fn post_message(&mut self, message: TargetedMessage) -> Result<(), String>; - /// Called when a message is to be posted to the parachain's relay chain. fn post_upward_message(&mut self, message: UpwardMessage) -> Result<(), String>; } diff --git a/polkadot/parachain/src/wasm_executor/validation_host.rs b/polkadot/parachain/src/wasm_executor/validation_host.rs index 711d223b41..4f74b3b25f 100644 --- a/polkadot/parachain/src/wasm_executor/validation_host.rs +++ b/polkadot/parachain/src/wasm_executor/validation_host.rs @@ -18,7 +18,7 @@ use std::{process, env, sync::Arc, sync::atomic, mem}; use codec::{Decode, Encode, EncodeAppend}; -use crate::{ValidationParams, ValidationResult, UpwardMessage, TargetedMessage}; +use crate::{ValidationParams, ValidationResult, UpwardMessage}; use super::{validate_candidate_internal, Error, Externalities}; use super::{MAX_CODE_MEM, MAX_RUNTIME_MEM}; use shared_memory::{SharedMem, SharedMemConf, EventState, WriteLockable, EventWait, EventSet}; @@ -44,7 +44,6 @@ pub const EXECUTION_TIMEOUT_SEC: u64 = 5; #[derive(Default)] struct WorkerExternalitiesInner { - egress_data: Vec, up_data: Vec, } @@ -54,15 +53,6 @@ struct WorkerExternalities { } impl Externalities for WorkerExternalities { - fn post_message(&mut self, message: TargetedMessage) -> Result<(), String> { - let mut inner = self.inner.lock(); - inner.egress_data = as EncodeAppend>::append_or_new( - mem::replace(&mut inner.egress_data, Vec::new()), - std::iter::once(message), - ).map_err(|e| e.what())?; - Ok(()) - } - fn post_upward_message(&mut self, message: UpwardMessage) -> Result<(), String> { let mut inner = self.inner.lock(); inner.up_data = as EncodeAppend>::append_or_new( @@ -141,9 +131,8 @@ pub fn run_worker(mem_id: &str) -> Result<(), String> { debug!("{} Candidate header: {:?}", process::id(), header); let (code, rest) = rest.split_at_mut(MAX_CODE_MEM); let (code, _) = code.split_at_mut(header.code_size as usize); - let (call_data, rest) = rest.split_at_mut(MAX_RUNTIME_MEM); + let (call_data, _) = rest.split_at_mut(MAX_RUNTIME_MEM); let (call_data, _) = call_data.split_at_mut(header.params_size as usize); - let message_data = rest; let result = validate_candidate_internal(code, call_data, worker_ext.clone()); debug!("{} Candidate validated: {:?}", process::id(), result); @@ -151,18 +140,12 @@ pub fn run_worker(mem_id: &str) -> Result<(), String> { match result { Ok(r) => { let inner = worker_ext.inner.lock(); - let egress_data = &inner.egress_data; - let e_len = egress_data.len(); let up_data = &inner.up_data; let up_len = up_data.len(); - if e_len + up_len > MAX_MESSAGE_MEM { + if up_len > MAX_MESSAGE_MEM { ValidationResultHeader::Error("Message data is too large".into()) } else { - message_data[0..e_len].copy_from_slice(egress_data); - - message_data[e_len..(e_len + up_len)].copy_from_slice(&up_data); - ValidationResultHeader::Ok(r) } }, @@ -334,14 +317,6 @@ impl ValidationHost { let header = ValidationResultHeader::decode(&mut header_buf).unwrap(); match header { ValidationResultHeader::Ok(result) => { - let egress = Vec::::decode(&mut message_data) - .map_err(|e| - Error::External( - format!("Could not decode egress messages: {}", e.what()) - ) - )?; - egress.into_iter().try_for_each(|msg| externalities.post_message(msg))?; - let upwards = Vec::::decode(&mut message_data) .map_err(|e| Error::External( diff --git a/polkadot/parachain/tests/lib.rs b/polkadot/parachain/tests/lib.rs index 4fa252ac58..762fba9d91 100644 --- a/polkadot/parachain/tests/lib.rs +++ b/polkadot/parachain/tests/lib.rs @@ -19,14 +19,11 @@ mod wasm_executor; use polkadot_parachain as parachain; use crate::parachain::{ - TargetedMessage, UpwardMessage, wasm_executor::{Externalities, run_worker}, + UpwardMessage, wasm_executor::{Externalities, run_worker}, }; struct DummyExt; impl Externalities for DummyExt { - fn post_message(&mut self, _: TargetedMessage) -> Result<(), String> { - Ok(()) - } fn post_upward_message(&mut self, _: UpwardMessage) -> Result<(), String> { Ok(()) } diff --git a/polkadot/primitives/src/parachain.rs b/polkadot/primitives/src/parachain.rs index 4c67ab11c0..045f99d623 100644 --- a/polkadot/primitives/src/parachain.rs +++ b/polkadot/primitives/src/parachain.rs @@ -35,7 +35,7 @@ use application_crypto::KeyTypeId; use trie::TrieConfiguration; pub use polkadot_parachain::{ - Id, ParachainDispatchOrigin, LOWEST_USER_ID, UpwardMessage, TargetedMessage, + Id, ParachainDispatchOrigin, LOWEST_USER_ID, UpwardMessage, }; /// The key type ID for a collator key. diff --git a/polkadot/validation/src/collation.rs b/polkadot/validation/src/collation.rs index a8ea572497..6812dda586 100644 --- a/polkadot/validation/src/collation.rs +++ b/polkadot/validation/src/collation.rs @@ -32,7 +32,7 @@ use polkadot_primitives::{ use polkadot_erasure_coding as erasure; use sp_api::ProvideRuntimeApi; use parachain::{ - wasm_executor::{self, ExecutionMode}, TargetedMessage, UpwardMessage, + wasm_executor::{self, ExecutionMode}, UpwardMessage, }; use trie::TrieConfiguration; use futures::prelude::*; @@ -174,34 +174,7 @@ pub fn message_queue_root>(messages: I) -> Hash trie::trie_types::Layout::::ordered_trie_root(messages) } -/// Compute the set of egress roots for all given outgoing messages. -pub fn egress_roots(outgoing: &mut [TargetedMessage]) -> Vec<(ParaId, Hash)> { - // stable sort messages by parachain ID. - outgoing.sort_by_key(|msg| ParaId::from(msg.target)); - - let mut egress_roots = Vec::new(); - { - let mut messages_iter = outgoing.iter().peekable(); - while let Some(batch_target) = messages_iter.peek().map(|o| o.target) { - // we borrow the iterator mutably to ensure it advances so the - // next iteration of the loop starts with `messages_iter` pointing to - // the next batch. - let messages_to = messages_iter - .clone() - .take_while(|o| o.target == batch_target) - .map(|o| { let _ = messages_iter.next(); &o.data[..] }); - - let computed_root = message_queue_root(messages_to); - egress_roots.push((batch_target, computed_root)); - } - } - - egress_roots -} - struct ExternalitiesInner { - parachain_index: ParaId, - outgoing: Vec, upward: Vec, fees_charged: Balance, free_balance: Balance, @@ -209,17 +182,6 @@ struct ExternalitiesInner { } impl wasm_executor::Externalities for ExternalitiesInner { - fn post_message(&mut self, message: TargetedMessage) -> Result<(), String> { - if message.target == self.parachain_index { - return Err("posted message to self".into()) - } - - self.apply_message_fee(message.data.len())?; - self.outgoing.push(message); - - Ok(()) - } - fn post_upward_message(&mut self, message: UpwardMessage) -> Result<(), String> { self.apply_message_fee(message.data.len())?; @@ -230,14 +192,12 @@ impl wasm_executor::Externalities for ExternalitiesInner { } impl ExternalitiesInner { - fn new(parachain_index: ParaId, free_balance: Balance, fee_schedule: FeeSchedule) -> Self { + fn new(free_balance: Balance, fee_schedule: FeeSchedule) -> Self { Self { - parachain_index, free_balance, fee_schedule, fees_charged: 0, upward: Vec::new(), - outgoing: Vec::new(), } } @@ -283,18 +243,14 @@ impl ExternalitiesInner { struct Externalities(Arc>); impl Externalities { - fn new(parachain_index: ParaId, free_balance: Balance, fee_schedule: FeeSchedule) -> Self { + fn new(free_balance: Balance, fee_schedule: FeeSchedule) -> Self { Self(Arc::new(Mutex::new( - ExternalitiesInner::new(parachain_index, free_balance, fee_schedule) + ExternalitiesInner::new(free_balance, fee_schedule) ))) } } impl wasm_executor::Externalities for Externalities { - fn post_message(&mut self, message: TargetedMessage) -> Result<(), String> { - self.0.lock().post_message(message) - } - fn post_upward_message(&mut self, message: UpwardMessage) -> Result<(), String> { self.0.lock().post_upward_message(message) } @@ -357,7 +313,7 @@ fn do_validation

( block_data: pov_block.block_data.0.clone(), }; - let ext = Externalities::new(para_id.clone(), chain_status.balance, chain_status.fee_schedule); + let ext = Externalities::new(chain_status.balance, chain_status.fee_schedule); match wasm_executor::validate_candidate( &validation_code, @@ -524,29 +480,9 @@ mod tests { use parachain::ParachainDispatchOrigin; use polkadot_primitives::parachain::{CandidateReceipt, HeadData}; - #[test] - fn ext_rejects_local_message() { - let mut ext = ExternalitiesInner { - parachain_index: 5.into(), - outgoing: Vec::new(), - upward: Vec::new(), - fees_charged: 0, - free_balance: 1_000_000, - fee_schedule: FeeSchedule { - base: 1000, - per_byte: 10, - }, - }; - - assert!(ext.post_message(TargetedMessage { target: 1.into(), data: Vec::new() }).is_ok()); - assert!(ext.post_message(TargetedMessage { target: 5.into(), data: Vec::new() }).is_err()); - } - #[test] fn ext_checks_upward_messages() { let ext = || ExternalitiesInner { - parachain_index: 5.into(), - outgoing: Vec::new(), upward: vec![ UpwardMessage{ data: vec![42], origin: ParachainDispatchOrigin::Parachain }, ], @@ -631,8 +567,6 @@ mod tests { #[test] fn ext_checks_fees_and_updates_correctly() { let mut ext = ExternalitiesInner { - parachain_index: 5.into(), - outgoing: Vec::new(), upward: vec![ UpwardMessage{ data: vec![42], origin: ParachainDispatchOrigin::Parachain }, ], @@ -647,20 +581,13 @@ mod tests { ext.apply_message_fee(100).unwrap(); assert_eq!(ext.fees_charged, 2000); - ext.post_message(TargetedMessage { - target: 1.into(), - data: vec![0u8; 100], - }).unwrap(); - assert_eq!(ext.fees_charged, 4000); - ext.post_upward_message(UpwardMessage { origin: ParachainDispatchOrigin::Signed, data: vec![0u8; 100], }).unwrap(); - assert_eq!(ext.fees_charged, 6000); + assert_eq!(ext.fees_charged, 4000); - - ext.apply_message_fee((1_000_000 - 6000 - 1000) / 10).unwrap(); + ext.apply_message_fee((1_000_000 - 4000 - 1000) / 10).unwrap(); assert_eq!(ext.fees_charged, 1_000_000); // cannot pay fee. diff --git a/polkadot/validation/src/lib.rs b/polkadot/validation/src/lib.rs index 34cdaa89ee..82041eb32d 100644 --- a/polkadot/validation/src/lib.rs +++ b/polkadot/validation/src/lib.rs @@ -48,7 +48,7 @@ use futures::prelude::*; pub use self::block_production::ProposerFactory; pub use self::collation::{ - validate_collation, message_queue_root, egress_roots, Collators, + validate_collation, message_queue_root, Collators, produce_receipt_and_chunks, }; pub use self::error::Error;