Skip to content

Commit

Permalink
[light-client] remove pending parentchain tx-inclusion check (#1455)
Browse files Browse the repository at this point in the history
* [itc-light-client] remove failing check for parentchain xt inclusion

* [itc-light-client] Add comment about justifying headers

* [itc-light-client] Just in case debug log unjustified headers

* [itc-light-client] typos in comments

* [itc-parentchain-block-importer] update obsolete documentation
  • Loading branch information
clangenb authored Sep 29, 2023
1 parent 2a1e1d0 commit 39cdc0d
Show file tree
Hide file tree
Showing 7 changed files with 12 additions and 93 deletions.
11 changes: 4 additions & 7 deletions core/parentchain/block-importer/src/block_importer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,10 @@ 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.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())
}
Expand Down
1 change: 0 additions & 1 deletion core/parentchain/light-client/src/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
4 changes: 0 additions & 4 deletions core/parentchain/light-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ where
{
fn submit_block(&mut self, signed_block: &SignedBlock<Block>) -> Result<(), Error>;

fn check_xt_inclusion(&mut self, block: &Block) -> Result<(), Error>;

fn get_state(&self) -> &LightValidationState<Block>;
}

Expand All @@ -85,8 +83,6 @@ pub trait ExtrinsicSender {
}

pub trait LightClientState<Block: ParentchainBlockTrait> {
fn num_xt_to_be_included(&self) -> Result<usize, Error>;

fn genesis_hash(&self) -> Result<HashFor<Block>, Error>;

fn latest_finalized_header(&self) -> Result<Block::Header, Error>;
Expand Down
65 changes: 5 additions & 60 deletions core/parentchain/light-client/src/light_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -115,7 +114,10 @@ impl<Block: ParentchainBlockTrait, OcallApi: EnclaveOnChainOCallApi>
}
}

// A valid grandpa proof proves finalization of all previous unjustified blocks.
// 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();
relay.push_header_hash(header.hash());

Expand All @@ -128,17 +130,6 @@ impl<Block: ParentchainBlockTrait, OcallApi: EnclaveOnChainOCallApi>

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<Block, OCallApi> Validator<Block> for LightValidation<Block, OCallApi>
Expand All @@ -160,44 +151,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| {
<HashingFor<Block>>::hash_of(xt) == <HashingFor<Block>>::hash_of(xt_opaque)
}) {
found_xts.push(index);
}
});

// sort highest index first
found_xts.sort_by(|a, b| b.cmp(a));

let rm: Vec<OpaqueExtrinsic> =
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<Block> {
&self.light_validation_state
}
Expand All @@ -210,10 +163,6 @@ where
OCallApi: EnclaveOnChainOCallApi,
{
fn send_extrinsics(&mut self, extrinsics: Vec<OpaqueExtrinsic>) -> 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| {
Expand All @@ -230,10 +179,6 @@ where
Block: ParentchainBlockTrait,
OCallApi: EnclaveOnChainOCallApi,
{
fn num_xt_to_be_included(&self) -> Result<usize, Error> {
self.light_validation_state.num_xt_to_be_included()
}

fn genesis_hash(&self) -> Result<HashFor<Block>, Error> {
self.light_validation_state.genesis_hash()
}
Expand Down
5 changes: 0 additions & 5 deletions core/parentchain/light-client/src/light_validation_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,6 @@ impl<Block> LightClientState<Block> for LightValidationState<Block>
where
Block: ParentchainBlockTrait,
{
fn num_xt_to_be_included(&self) -> Result<usize, Error> {
let relay = self.get_relay();
Ok(relay.verify_tx_inclusion.len())
}

fn genesis_hash(&self) -> Result<HashFor<Block>, Error> {
Ok(self.get_relay().genesis_hash)
}
Expand Down
8 changes: 0 additions & 8 deletions core/parentchain/light-client/src/mocks/validator_mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ impl Validator<Block> for ValidatorMock {
Ok(())
}

fn check_xt_inclusion(&mut self, _block: &Block) -> Result<()> {
Ok(())
}

fn get_state(&self) -> &LightValidationState<Block> {
&self.light_validation_state
}
Expand All @@ -65,10 +61,6 @@ impl ExtrinsicSender for ValidatorMock {
}

impl LightClientState<Block> for ValidatorMock {
fn num_xt_to_be_included(&self) -> Result<usize> {
todo!()
}

fn genesis_hash(&self) -> Result<HashFor<Block>> {
todo!()
}
Expand Down
11 changes: 3 additions & 8 deletions core/parentchain/light-client/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -35,7 +32,6 @@ pub struct RelayState<Block: BlockT> {
pub current_validator_set_id: SetId,
header_hashes: VecDeque<Block::Hash>,
pub unjustified_headers: Vec<Block::Hash>, // Finalized headers without grandpa proof
pub verify_tx_inclusion: Vec<OpaqueExtrinsic>, // Transactions sent by the relay
pub scheduled_change: Option<ScheduledChangeAtBlock<Block::Header>>, // Scheduled Authorities change as indicated in the header's digest.
}

Expand Down Expand Up @@ -79,7 +75,6 @@ impl<Block: BlockT> RelayState<Block> {
current_validator_set: validator_set,
current_validator_set_id: 0,
unjustified_headers: Vec::new(),
verify_tx_inclusion: Vec::new(),
scheduled_change: None,
}
}
Expand All @@ -95,11 +90,11 @@ impl<Block: BlockT> fmt::Debug for RelayState<Block> {
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: {}, number of unjustified headers: {} }}",
self.last_finalized_block_header.number(),
self.current_validator_set,
self.current_validator_set_id,
self.verify_tx_inclusion.len()
self.unjustified_headers.len()
)
}
}

0 comments on commit 39cdc0d

Please sign in to comment.