From 78eb3ee62f2e0e6c9140dbdfeb4fd780f333420c Mon Sep 17 00:00:00 2001 From: Christian Langenbacher Date: Tue, 19 Sep 2023 09:18:46 +0200 Subject: [PATCH 1/5] [itc-light-client] remove failing check for parentchain xt inclusion --- .../block-importer/src/block_importer.rs | 9 ++- core/parentchain/light-client/src/io.rs | 1 - core/parentchain/light-client/src/lib.rs | 4 -- .../light-client/src/light_validation.rs | 60 +------------------ .../src/light_validation_state.rs | 5 -- .../light-client/src/mocks/validator_mock.rs | 8 --- core/parentchain/light-client/src/state.rs | 10 +--- 7 files changed, 7 insertions(+), 90 deletions(-) diff --git a/core/parentchain/block-importer/src/block_importer.rs b/core/parentchain/block-importer/src/block_importer.rs index f3130c4143..c78c34052d 100644 --- a/core/parentchain/block-importer/src/block_importer.rs +++ b/core/parentchain/block-importer/src/block_importer.rs @@ -116,11 +116,10 @@ impl< { // Check if there are any extrinsics in the to-be-imported block that we sent and cached in the light-client before. // If so, remove them now from the cache. - if let Err(e) = self.validator_accessor.execute_mut_on_validator(|v| { - v.check_xt_inclusion(&signed_block.block)?; - - v.submit_block(&signed_block) - }) { + if let Err(e) = self + .validator_accessor + .execute_mut_on_validator(|v| v.submit_block(&signed_block)) + { error!("[{:?}] Header submission to light client failed: {:?}", id, e); return Err(e.into()) } diff --git a/core/parentchain/light-client/src/io.rs b/core/parentchain/light-client/src/io.rs index 99145f5174..4b95a38965 100644 --- a/core/parentchain/light-client/src/io.rs +++ b/core/parentchain/light-client/src/io.rs @@ -358,7 +358,6 @@ pub mod sgx_tests { .unwrap(); assert_eq!(validator.genesis_hash().unwrap(), parachain_params.genesis_header.hash()); - assert_eq!(validator.num_xt_to_be_included().unwrap(), 0); assert_eq!(validator.latest_finalized_header().unwrap(), parachain_params.genesis_header); assert_eq!( validator.penultimate_finalized_block_header().unwrap(), diff --git a/core/parentchain/light-client/src/lib.rs b/core/parentchain/light-client/src/lib.rs index 100efa41ef..00579b2e09 100644 --- a/core/parentchain/light-client/src/lib.rs +++ b/core/parentchain/light-client/src/lib.rs @@ -74,8 +74,6 @@ where { fn submit_block(&mut self, signed_block: &SignedBlock) -> Result<(), Error>; - fn check_xt_inclusion(&mut self, block: &Block) -> Result<(), Error>; - fn get_state(&self) -> &LightValidationState; } @@ -85,8 +83,6 @@ pub trait ExtrinsicSender { } pub trait LightClientState { - fn num_xt_to_be_included(&self) -> Result; - fn genesis_hash(&self) -> Result, Error>; fn latest_finalized_header(&self) -> Result; diff --git a/core/parentchain/light-client/src/light_validation.rs b/core/parentchain/light-client/src/light_validation.rs index 290419c4ec..d92e2bad3b 100644 --- a/core/parentchain/light-client/src/light_validation.rs +++ b/core/parentchain/light-client/src/light_validation.rs @@ -26,10 +26,9 @@ use core::iter::Iterator; use itp_ocall_api::EnclaveOnChainOCallApi; use itp_storage::{Error as StorageError, StorageProof, StorageProofChecker}; use itp_types::parentchain::{IdentifyParentchain, ParentchainId}; -use log::*; use sp_runtime::{ generic::SignedBlock, - traits::{Block as ParentchainBlockTrait, Hash as HashTrait, Header as HeaderTrait}, + traits::{Block as ParentchainBlockTrait, Header as HeaderTrait}, Justifications, OpaqueExtrinsic, }; use std::{boxed::Box, fmt, sync::Arc, vec::Vec}; @@ -128,17 +127,6 @@ impl Ok(()) } - - fn submit_xt_to_be_included(&mut self, extrinsic: OpaqueExtrinsic) { - let relay = self.light_validation_state.get_relay_mut(); - relay.verify_tx_inclusion.push(extrinsic); - - debug!( - "[{:?}] {} extrinsics in cache, waiting for inclusion verification", - self.parentchain_id, - relay.verify_tx_inclusion.len() - ); - } } impl Validator for LightValidation @@ -160,44 +148,6 @@ where self.submit_finalized_headers(header.clone(), vec![], justifications) } - fn check_xt_inclusion(&mut self, block: &Block) -> Result<(), Error> { - let relay = self.light_validation_state.get_relay_mut(); - - if relay.verify_tx_inclusion.is_empty() { - return Ok(()) - } - - let mut found_xts = vec![]; - block.extrinsics().iter().for_each(|xt| { - if let Some(index) = relay.verify_tx_inclusion.iter().position(|xt_opaque| { - >::hash_of(xt) == >::hash_of(xt_opaque) - }) { - found_xts.push(index); - } - }); - - // sort highest index first - found_xts.sort_by(|a, b| b.cmp(a)); - - let rm: Vec = - found_xts.into_iter().map(|i| relay.verify_tx_inclusion.remove(i)).collect(); - - if !rm.is_empty() { - info!( - "[{:?}] Verified inclusion proof of {} extrinsics.", - self.parentchain_id, - rm.len() - ); - } - debug!( - "[{:?}] {} extrinsics remaining in cache, waiting for inclusion verification", - self.parentchain_id, - relay.verify_tx_inclusion.len() - ); - - Ok(()) - } - fn get_state(&self) -> &LightValidationState { &self.light_validation_state } @@ -210,10 +160,6 @@ where OCallApi: EnclaveOnChainOCallApi, { fn send_extrinsics(&mut self, extrinsics: Vec) -> Result<(), Error> { - for xt in extrinsics.iter() { - self.submit_xt_to_be_included(xt.clone()); - } - self.ocall_api .send_to_parentchain(extrinsics, &self.parentchain_id) .map_err(|e| { @@ -230,10 +176,6 @@ where Block: ParentchainBlockTrait, OCallApi: EnclaveOnChainOCallApi, { - fn num_xt_to_be_included(&self) -> Result { - self.light_validation_state.num_xt_to_be_included() - } - fn genesis_hash(&self) -> Result, Error> { self.light_validation_state.genesis_hash() } diff --git a/core/parentchain/light-client/src/light_validation_state.rs b/core/parentchain/light-client/src/light_validation_state.rs index b280d05817..b86a242677 100644 --- a/core/parentchain/light-client/src/light_validation_state.rs +++ b/core/parentchain/light-client/src/light_validation_state.rs @@ -52,11 +52,6 @@ impl LightClientState for LightValidationState where Block: ParentchainBlockTrait, { - fn num_xt_to_be_included(&self) -> Result { - let relay = self.get_relay(); - Ok(relay.verify_tx_inclusion.len()) - } - fn genesis_hash(&self) -> Result, Error> { Ok(self.get_relay().genesis_hash) } diff --git a/core/parentchain/light-client/src/mocks/validator_mock.rs b/core/parentchain/light-client/src/mocks/validator_mock.rs index 7798b7dba3..fcd1e4c3da 100644 --- a/core/parentchain/light-client/src/mocks/validator_mock.rs +++ b/core/parentchain/light-client/src/mocks/validator_mock.rs @@ -49,10 +49,6 @@ impl Validator for ValidatorMock { Ok(()) } - fn check_xt_inclusion(&mut self, _block: &Block) -> Result<()> { - Ok(()) - } - fn get_state(&self) -> &LightValidationState { &self.light_validation_state } @@ -65,10 +61,6 @@ impl ExtrinsicSender for ValidatorMock { } impl LightClientState for ValidatorMock { - fn num_xt_to_be_included(&self) -> Result { - todo!() - } - fn genesis_hash(&self) -> Result> { todo!() } diff --git a/core/parentchain/light-client/src/state.rs b/core/parentchain/light-client/src/state.rs index 1fef61780c..130307ef4d 100644 --- a/core/parentchain/light-client/src/state.rs +++ b/core/parentchain/light-client/src/state.rs @@ -17,10 +17,7 @@ use codec::{Decode, Encode}; use sp_consensus_grandpa::{AuthorityList, SetId}; -use sp_runtime::{ - traits::{Block as BlockT, Header as HeaderT}, - OpaqueExtrinsic, -}; +use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use std::{collections::VecDeque, fmt, vec::Vec}; /// Defines the amount of parentchain headers to keep. @@ -35,7 +32,6 @@ pub struct RelayState { pub current_validator_set_id: SetId, header_hashes: VecDeque, pub unjustified_headers: Vec, // Finalized headers without grandpa proof - pub verify_tx_inclusion: Vec, // Transactions sent by the relay pub scheduled_change: Option>, // Scheduled Authorities change as indicated in the header's digest. } @@ -79,7 +75,6 @@ impl RelayState { current_validator_set: validator_set, current_validator_set_id: 0, unjustified_headers: Vec::new(), - verify_tx_inclusion: Vec::new(), scheduled_change: None, } } @@ -95,11 +90,10 @@ impl fmt::Debug for RelayState { write!( f, "RelayInfo {{ last_finalized_block_header_number: {:?}, current_validator_set: {:?}, \ - current_validator_set_id: {} amount of transaction in tx_inclusion_queue: {} }}", + current_validator_set_id: {} }}", self.last_finalized_block_header.number(), self.current_validator_set, self.current_validator_set_id, - self.verify_tx_inclusion.len() ) } } From fbff27d091d75157bfcb6db4c395459aa1077101 Mon Sep 17 00:00:00 2001 From: Christian Langenbacher Date: Tue, 19 Sep 2023 09:24:30 +0200 Subject: [PATCH 2/5] [itc-light-client] Add comment about justifying headers --- core/parentchain/light-client/src/light_validation.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/parentchain/light-client/src/light_validation.rs b/core/parentchain/light-client/src/light_validation.rs index d92e2bad3b..abc9bcc3d3 100644 --- a/core/parentchain/light-client/src/light_validation.rs +++ b/core/parentchain/light-client/src/light_validation.rs @@ -114,7 +114,10 @@ impl } } - // A valid grandpa proof proves finalization of all previous unjustified blocks. + // Todo: Justifying the headers here is actually wrong, but it prevents ever growing + // `unjustified_headers` queue because in the parachain case we won't have justifications, + // and in solo chain setups we only get a justification upon an Grandpa authority change. + // Hence, we justify the headers here until we properly solve this in #1404. relay.justify_headers(); relay.push_header_hash(header.hash()); From 9582d4ed10206d6c451b3decabc67b69c2ada651 Mon Sep 17 00:00:00 2001 From: Christian Langenbacher Date: Tue, 19 Sep 2023 09:26:28 +0200 Subject: [PATCH 3/5] [itc-light-client] Just in case debug log unjustified headers --- core/parentchain/light-client/src/state.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/parentchain/light-client/src/state.rs b/core/parentchain/light-client/src/state.rs index 130307ef4d..e21f86e2e4 100644 --- a/core/parentchain/light-client/src/state.rs +++ b/core/parentchain/light-client/src/state.rs @@ -90,10 +90,11 @@ impl fmt::Debug for RelayState { write!( f, "RelayInfo {{ last_finalized_block_header_number: {:?}, current_validator_set: {:?}, \ - current_validator_set_id: {} }}", + current_validator_set_id: {}, number of unjustified headers: {} }}", self.last_finalized_block_header.number(), self.current_validator_set, self.current_validator_set_id, + self.unjustified_headers.len() ) } } From 73214efef2c7f430e6df38dc81e8d78a3960cc27 Mon Sep 17 00:00:00 2001 From: Christian Langenbacher Date: Tue, 19 Sep 2023 09:31:40 +0200 Subject: [PATCH 4/5] [itc-light-client] typos in comments --- core/parentchain/light-client/src/light_validation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/parentchain/light-client/src/light_validation.rs b/core/parentchain/light-client/src/light_validation.rs index abc9bcc3d3..3eda5c8490 100644 --- a/core/parentchain/light-client/src/light_validation.rs +++ b/core/parentchain/light-client/src/light_validation.rs @@ -114,8 +114,8 @@ impl } } - // Todo: Justifying the headers here is actually wrong, but it prevents ever growing - // `unjustified_headers` queue because in the parachain case we won't have justifications, + // Todo: Justifying the headers here is actually wrong, but it prevents an ever-growing + // `unjustified_headers` queue because in the parachain case we won't have justifications, // and in solo chain setups we only get a justification upon an Grandpa authority change. // Hence, we justify the headers here until we properly solve this in #1404. relay.justify_headers(); From 03d1c15840bae5de949c0aef3e4188247a5a9dac Mon Sep 17 00:00:00 2001 From: Christian Langenbacher Date: Mon, 25 Sep 2023 11:40:00 +0200 Subject: [PATCH 5/5] [itc-parentchain-block-importer] update obsolete documentation --- core/parentchain/block-importer/src/block_importer.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/parentchain/block-importer/src/block_importer.rs b/core/parentchain/block-importer/src/block_importer.rs index c78c34052d..2e7e60dc0f 100644 --- a/core/parentchain/block-importer/src/block_importer.rs +++ b/core/parentchain/block-importer/src/block_importer.rs @@ -114,8 +114,6 @@ impl< for (signed_block, raw_events) in blocks_to_import.into_iter().zip(events_to_import.into_iter()) { - // Check if there are any extrinsics in the to-be-imported block that we sent and cached in the light-client before. - // If so, remove them now from the cache. if let Err(e) = self .validator_accessor .execute_mut_on_validator(|v| v.submit_block(&signed_block))