From 36b7037c6c0a587c11f9f096b2893a5755dd64ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 21 Feb 2024 17:17:54 +0100 Subject: [PATCH] Check that the validation code matches the parachain code This introduces a check to ensure that the parachain code matches the validation code stored in the relay chain state. If not, it will print a warning. This should be mainly useful for parachain builders to make sure they have setup everything correctly. --- .../consensus/aura/src/collators/basic.rs | 22 +++++++- .../consensus/aura/src/collators/lookahead.rs | 17 ++++-- .../consensus/aura/src/collators/mod.rs | 55 +++++++++++++++++++ cumulus/client/consensus/common/src/tests.rs | 9 +++ cumulus/client/network/src/tests.rs | 9 +++ .../src/lib.rs | 15 ++++- .../client/relay-chain-interface/src/lib.rs | 22 +++++++- .../relay-chain-rpc-interface/src/lib.rs | 13 ++++- .../src/rpc_client.rs | 14 +++++ polkadot/node/core/backing/src/lib.rs | 10 ++-- 10 files changed, 173 insertions(+), 13 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs index 8740b06005d6..52b83254951f 100644 --- a/cumulus/client/consensus/aura/src/collators/basic.rs +++ b/cumulus/client/consensus/aura/src/collators/basic.rs @@ -33,12 +33,12 @@ use cumulus_relay_chain_interface::RelayChainInterface; use polkadot_node_primitives::CollationResult; use polkadot_overseer::Handle as OverseerHandle; -use polkadot_primitives::{CollatorPair, Id as ParaId}; +use polkadot_primitives::{CollatorPair, Id as ParaId, ValidationCode}; use futures::{channel::mpsc::Receiver, prelude::*}; use sc_client_api::{backend::AuxStore, BlockBackend, BlockOf}; use sc_consensus::BlockImport; -use sp_api::ProvideRuntimeApi; +use sp_api::{CallApiAt, ProvideRuntimeApi}; use sp_application_crypto::AppPublic; use sp_blockchain::HeaderBackend; use sp_consensus::SyncOracle; @@ -47,6 +47,7 @@ use sp_core::crypto::Pair; use sp_inherents::CreateInherentDataProviders; use sp_keystore::KeystorePtr; use sp_runtime::traits::{Block as BlockT, Header as HeaderT, Member}; +use sp_state_machine::Backend as _; use std::{convert::TryFrom, sync::Arc, time::Duration}; use crate::collator as collator_util; @@ -100,6 +101,7 @@ where + AuxStore + HeaderBackend + BlockBackend + + CallApiAt + Send + Sync + 'static, @@ -172,6 +174,22 @@ where continue } + let Ok(Some(code)) = + params.para_client.state_at(parent_hash).map_err(drop).and_then(|s| { + s.storage(&sp_core::storage::well_known_keys::CODE).map_err(drop) + }) + else { + continue; + }; + + super::check_validation_code_or_log( + &ValidationCode::from(code).hash(), + params.para_id, + ¶ms.relay_client, + *request.relay_parent(), + ) + .await; + let relay_parent_header = match params.relay_client.header(RBlockId::hash(*request.relay_parent())).await { Err(e) => reject_with_error!(e), diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index a9f33173d832..161f10d55a19 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -290,10 +290,7 @@ where // If the longest chain has space, build upon that. Otherwise, don't // build at all. potential_parents.sort_by_key(|a| a.depth); - let initial_parent = match potential_parents.pop() { - None => continue, - Some(p) => p, - }; + let Some(initial_parent) = potential_parents.pop() else { continue }; // Build in a loop until not allowed. Note that the authorities can change // at any block, so we need to re-claim our slot every time. @@ -301,6 +298,10 @@ where let mut parent_header = initial_parent.header; let overseer_handle = &mut params.overseer_handle; + // We mainly call this to inform users at genesis if there is a mismatch with the + // on-chain data. + collator.collator_service().check_block_status(parent_hash, &parent_header); + // This needs to change to support elastic scaling, but for continuously // scheduled chains this ensures that the backlog will grow steadily. for n_built in 0..2 { @@ -353,6 +354,14 @@ where Some(v) => v, }; + super::check_validation_code_or_log( + &validation_code_hash, + params.para_id, + ¶ms.relay_client, + relay_parent, + ) + .await; + match collator .collate( &parent_header, diff --git a/cumulus/client/consensus/aura/src/collators/mod.rs b/cumulus/client/consensus/aura/src/collators/mod.rs index 4c7b759daf73..6e0067d0cedb 100644 --- a/cumulus/client/consensus/aura/src/collators/mod.rs +++ b/cumulus/client/consensus/aura/src/collators/mod.rs @@ -20,5 +20,60 @@ //! included parachain block, as well as the [`lookahead`] collator, which prospectively //! builds on parachain blocks which have not yet been included in the relay chain. +use cumulus_relay_chain_interface::RelayChainInterface; +use polkadot_primitives::{ + Hash as RHash, Id as ParaId, OccupiedCoreAssumption, ValidationCodeHash, +}; + pub mod basic; pub mod lookahead; + +/// Check the `local_validation_code_hash` against the validation code hash in the relay chain +/// state. +/// +/// If the code hashes do not match, it prints a warning. +async fn check_validation_code_or_log( + local_validation_code_hash: &ValidationCodeHash, + para_id: ParaId, + relay_client: &impl RelayChainInterface, + relay_parent: RHash, +) { + let state_validation_code_hash = match relay_client + .validation_code_hash(relay_parent, para_id, OccupiedCoreAssumption::Included) + .await + { + Ok(hash) => hash, + Err(error) => { + tracing::debug!( + target: super::LOG_TARGET, + %error, + ?relay_parent, + %para_id, + "Failed to fetch validation code hash", + ); + return + }, + }; + + match state_validation_code_hash { + Some(state) => + if state != *local_validation_code_hash { + tracing::warn!( + target: super::LOG_TARGET, + %para_id, + ?relay_parent, + ?local_validation_code_hash, + relay_validation_code_hash = ?state, + "Parachain code doesn't match validation code stored in the relay chain state", + ); + }, + None => { + tracing::warn!( + target: super::LOG_TARGET, + %para_id, + ?relay_parent, + "Could not find validation code for parachain in the relay chain state.", + ); + }, + } +} diff --git a/cumulus/client/consensus/common/src/tests.rs b/cumulus/client/consensus/common/src/tests.rs index 597d1ab2acc2..bfb95ae388ae 100644 --- a/cumulus/client/consensus/common/src/tests.rs +++ b/cumulus/client/consensus/common/src/tests.rs @@ -136,6 +136,15 @@ impl RelayChainInterface for Relaychain { Ok(Some(PersistedValidationData { parent_head, ..Default::default() })) } + async fn validation_code_hash( + &self, + _: PHash, + _: ParaId, + _: OccupiedCoreAssumption, + ) -> RelayChainResult> { + unimplemented!("Not needed for test") + } + async fn candidate_pending_availability( &self, _: PHash, diff --git a/cumulus/client/network/src/tests.rs b/cumulus/client/network/src/tests.rs index e03f470753bb..d986635f961c 100644 --- a/cumulus/client/network/src/tests.rs +++ b/cumulus/client/network/src/tests.rs @@ -117,6 +117,15 @@ impl RelayChainInterface for DummyRelayChainInterface { })) } + async fn validation_code_hash( + &self, + _: PHash, + _: ParaId, + _: OccupiedCoreAssumption, + ) -> RelayChainResult> { + unimplemented!("Not needed for test") + } + async fn candidate_pending_availability( &self, _: PHash, diff --git a/cumulus/client/relay-chain-inprocess-interface/src/lib.rs b/cumulus/client/relay-chain-inprocess-interface/src/lib.rs index 866214fe2c52..6ea02b2e7c1f 100644 --- a/cumulus/client/relay-chain-inprocess-interface/src/lib.rs +++ b/cumulus/client/relay-chain-inprocess-interface/src/lib.rs @@ -21,7 +21,7 @@ use cumulus_primitives_core::{ relay_chain::{ runtime_api::ParachainHost, Block as PBlock, BlockId, CommittedCandidateReceipt, Hash as PHash, Header as PHeader, InboundHrmpMessage, OccupiedCoreAssumption, SessionIndex, - ValidatorId, + ValidationCodeHash, ValidatorId, }, InboundDownwardMessage, ParaId, PersistedValidationData, }; @@ -115,6 +115,19 @@ impl RelayChainInterface for RelayChainInProcessInterface { )?) } + async fn validation_code_hash( + &self, + hash: PHash, + para_id: ParaId, + occupied_core_assumption: OccupiedCoreAssumption, + ) -> RelayChainResult> { + Ok(self.full_client.runtime_api().validation_code_hash( + hash, + para_id, + occupied_core_assumption, + )?) + } + async fn candidate_pending_availability( &self, hash: PHash, diff --git a/cumulus/client/relay-chain-interface/src/lib.rs b/cumulus/client/relay-chain-interface/src/lib.rs index de5e7891b30d..bb93e6a168c8 100644 --- a/cumulus/client/relay-chain-interface/src/lib.rs +++ b/cumulus/client/relay-chain-interface/src/lib.rs @@ -30,7 +30,7 @@ use cumulus_primitives_core::relay_chain::BlockId; pub use cumulus_primitives_core::{ relay_chain::{ CommittedCandidateReceipt, Hash as PHash, Header as PHeader, InboundHrmpMessage, - OccupiedCoreAssumption, SessionIndex, ValidatorId, + OccupiedCoreAssumption, SessionIndex, ValidationCodeHash, ValidatorId, }, InboundDownwardMessage, ParaId, PersistedValidationData, }; @@ -194,6 +194,15 @@ pub trait RelayChainInterface: Send + Sync { relay_parent: PHash, relevant_keys: &Vec>, ) -> RelayChainResult; + + /// Returns the validation code hash for the given `para_id` using the given + /// `occupied_core_assumption`. + async fn validation_code_hash( + &self, + relay_parent: PHash, + para_id: ParaId, + occupied_core_assumption: OccupiedCoreAssumption, + ) -> RelayChainResult>; } #[async_trait] @@ -301,4 +310,15 @@ where async fn header(&self, block_id: BlockId) -> RelayChainResult> { (**self).header(block_id).await } + + async fn validation_code_hash( + &self, + relay_parent: PHash, + para_id: ParaId, + occupied_core_assumption: OccupiedCoreAssumption, + ) -> RelayChainResult> { + (**self) + .validation_code_hash(relay_parent, para_id, occupied_core_assumption) + .await + } } diff --git a/cumulus/client/relay-chain-rpc-interface/src/lib.rs b/cumulus/client/relay-chain-rpc-interface/src/lib.rs index 96f8fc8b5563..3a4c186e301e 100644 --- a/cumulus/client/relay-chain-rpc-interface/src/lib.rs +++ b/cumulus/client/relay-chain-rpc-interface/src/lib.rs @@ -19,7 +19,7 @@ use core::time::Duration; use cumulus_primitives_core::{ relay_chain::{ CommittedCandidateReceipt, Hash as RelayHash, Header as RelayHeader, InboundHrmpMessage, - OccupiedCoreAssumption, SessionIndex, ValidatorId, + OccupiedCoreAssumption, SessionIndex, ValidationCodeHash, ValidatorId, }, InboundDownwardMessage, ParaId, PersistedValidationData, }; @@ -110,6 +110,17 @@ impl RelayChainInterface for RelayChainRpcInterface { .await } + async fn validation_code_hash( + &self, + hash: RelayHash, + para_id: ParaId, + occupied_core_assumption: OccupiedCoreAssumption, + ) -> RelayChainResult> { + self.rpc_client + .validation_code_hash(hash, para_id, occupied_core_assumption) + .await + } + async fn candidate_pending_availability( &self, hash: RelayHash, diff --git a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs index a912997e947d..6578210a259c 100644 --- a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs +++ b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs @@ -647,6 +647,20 @@ impl RelayChainRpcClient { .await } + pub async fn validation_code_hash( + &self, + at: RelayHash, + para_id: ParaId, + occupied_core_assumption: OccupiedCoreAssumption, + ) -> Result, RelayChainError> { + self.call_remote_runtime_function( + "ParachainHost_validation_code_hash", + at, + Some((para_id, occupied_core_assumption)), + ) + .await + } + fn send_register_message_to_worker( &self, message: RpcDispatcherMessage, diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 98bbd6232add..fa225a1dbcdb 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -1722,18 +1722,20 @@ async fn background_validate_and_make_available( if rp_state.awaiting_validation.insert(candidate_hash) { // spawn background task. let bg = async move { - if let Err(e) = validate_and_make_available(params).await { - if let Error::BackgroundValidationMpsc(error) = e { + if let Err(error) = validate_and_make_available(params).await { + if let Error::BackgroundValidationMpsc(error) = error { gum::debug!( target: LOG_TARGET, + ?candidate_hash, ?error, "Mpsc background validation mpsc died during validation- leaf no longer active?" ); } else { gum::error!( target: LOG_TARGET, - "Failed to validate and make available: {:?}", - e + ?candidate_hash, + ?error, + "Failed to validate and make available", ); } }