Minor light-client refactor (#1287)

* [itc-light-client] refactor

* [itc-light-client] remove obsolete `NoSuchRelayExists` error

* [itc-light-client] make `submit_xt_to_be_included` infallible and remove expect statement.

* [itc-light-client] remove error that is not used yet.

* [itc-light-client] Remove `Default` derive, which can't be satisfied

core/offchain-worker-executor/src/executor.rs

@@ -155,7 +155,7 @@ impl<
 
 	fn get_latest_parentchain_header(&self) -> Result<ParentchainBlock::Header> {
 		let header = self.validator_accessor.execute_on_validator(|v| {
-			let latest_parentchain_header = v.latest_finalized_header(v.num_relays())?;
+			let latest_parentchain_header = v.latest_finalized_header()?;
 			Ok(latest_parentchain_header)
 		})?;
 		Ok(header)

core/parentchain/block-importer/src/block_importer.rs

@@ -21,8 +21,7 @@ use crate::{error::Result, ImportParentchainBlocks};
 use ita_stf::ParentchainHeader;
 use itc_parentchain_indirect_calls_executor::ExecuteIndirectCalls;
 use itc_parentchain_light_client::{
-	concurrent_access::ValidatorAccess, BlockNumberOps, ExtrinsicSender, LightClientState,
-	Validator,
+	concurrent_access::ValidatorAccess, BlockNumberOps, ExtrinsicSender, Validator,
 };
 use itp_extrinsics_factory::CreateExtrinsics;
 use itp_stf_executor::traits::StfUpdateState;
@@ -127,9 +126,9 @@ 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(v.num_relays(), &signed_block.block)?;
+				v.check_xt_inclusion(&signed_block.block)?;
 
-				v.submit_block(v.num_relays(), &signed_block)
+				v.submit_block(&signed_block)
 			}) {
 				error!("[Validator] Header submission failed: {:?}", e);
 				return Err(e.into())

core/parentchain/light-client/src/concurrent_access.rs

@@ -105,10 +105,8 @@ where
 	where
 		F: FnOnce(&Self::ValidatorType) -> Result<R>,
 	{
-		let mut light_validation_lock =
+		let light_validation_lock =
 			self.light_validation.write().map_err(|_| Error::PoisonedLock)?;
-		let state = Seal::unseal_from_static_file()?;
-		light_validation_lock.set_state(state);
 		getter_function(&light_validation_lock)
 	}
 
@@ -118,8 +116,6 @@ where
 	{
 		let mut light_validation_lock =
 			self.light_validation.write().map_err(|_| Error::PoisonedLock)?;
-		let state = Seal::unseal_from_static_file()?;
-		light_validation_lock.set_state(state);
 		let result = mutating_function(&mut light_validation_lock);
 		Seal::seal_to_static_file(light_validation_lock.get_state())?;
 		result

core/parentchain/light-client/src/error.rs

@@ -43,8 +43,6 @@ pub enum Error {
 	ValidatorSetMismatch,
 	#[error("Invalid ancestry proof")]
 	InvalidAncestryProof,
-	#[error("No such relay exists")]
-	NoSuchRelayExists,
 	#[error("Invalid Finality Proof: {0}")]
 	InvalidFinalityProof(#[from] JustificationError),
 	#[error("Header ancestry mismatch")]

core/parentchain/light-client/src/io.rs

@@ -19,7 +19,8 @@ use crate::{
 	error::Result,
 	finality::{Finality, GrandpaFinality, ParachainFinality},
 	light_client_init_params::{GrandpaParams, SimpleParams},
-	light_validation::LightValidation,
+	light_validation::{check_validator_set_proof, LightValidation},
+	state::RelayState,
 	Error, LightValidationState, NumberFor, Validator,
 };
 use codec::{Decode, Encode};
@@ -28,7 +29,6 @@ use itp_ocall_api::EnclaveOnChainOCallApi;
 use itp_settings::files::LIGHT_CLIENT_DB;
 use itp_sgx_io::{seal, unseal, StaticSealedIO};
 use log::*;
-use sp_finality_grandpa::AuthorityList;
 use sp_runtime::traits::{Block, Header};
 use std::{boxed::Box, fs, sgxfs::SgxFile, sync::Arc};
 
@@ -68,26 +68,43 @@ where
 	NumberFor<B>: finality_grandpa::BlockNumberOps,
 	OCallApi: EnclaveOnChainOCallApi,
 {
+	check_validator_set_proof::<B>(
+		params.genesis_header.state_root(),
+		params.authority_proof,
+		&params.authorities,
+	)?;
+
 	// FIXME: That should be an unique path.
 	if SgxFile::open(LIGHT_CLIENT_DB).is_err() {
 		info!("[Enclave] ChainRelay DB not found, creating new! {}", LIGHT_CLIENT_DB);
-		return init_grandpa_validator::<B, OCallApi>(params, ocall_api)
+		let validator = init_grandpa_validator::<B, OCallApi>(
+			ocall_api,
+			RelayState::new(params.genesis_header, params.authorities).into(),
+		)?;
+		LightClientStateSeal::<B, LightValidationState<B>>::seal_to_static_file(
+			validator.get_state(),
+		)?;
+		return Ok(validator)
 	}
 
 	let (validation_state, genesis_hash) = get_validation_state::<B>()?;
 
-	let mut validator = init_grandpa_validator::<B, OCallApi>(params.clone(), ocall_api)?;
-
-	if genesis_hash == params.genesis_header.hash() {
-		validator.set_state(validation_state);
-		// The init_grandpa_validator function clear the state every time,
-		// so we should write the state again.
-		LightClientStateSeal::<B, LightValidationState<B>>::seal_to_static_file(
-			validator.get_state(),
-		)?;
+	let init_state = if genesis_hash == params.genesis_header.hash() {
 		info!("Found already initialized light client with Genesis Hash: {:?}", genesis_hash);
-	}
+		validation_state
+	} else {
+		info!(
+			"Previous light client db belongs to another parentchain genesis. Creating new: {:?}",
+			genesis_hash
+		);
+		RelayState::new(params.genesis_header, params.authorities).into()
+	};
+
+	let validator = init_grandpa_validator::<B, OCallApi>(ocall_api, init_state)?;
+
 	info!("light client state: {:?}", validator);
+
+	LightClientStateSeal::<B, LightValidationState<B>>::seal_to_static_file(validator.get_state())?;
 	Ok(validator)
 }
 
@@ -103,78 +120,76 @@ where
 	// FIXME: That should be an unique path.
 	if SgxFile::open(LIGHT_CLIENT_DB).is_err() {
 		info!("[Enclave] ChainRelay DB not found, creating new! {}", LIGHT_CLIENT_DB);
-		return init_parachain_validator::<B, OCallApi>(params, ocall_api)
+		let validator = init_parachain_validator::<B, OCallApi>(
+			ocall_api,
+			RelayState::new(params.genesis_header, Default::default()).into(),
+		)?;
+		LightClientStateSeal::<B, LightValidationState<B>>::seal_to_static_file(
+			validator.get_state(),
+		)?;
+		return Ok(validator)
 	}
 
 	let (validation_state, genesis_hash) = get_validation_state::<B>()?;
 
-	let mut validator = init_parachain_validator::<B, OCallApi>(params.clone(), ocall_api)?;
-
-	if genesis_hash == params.genesis_header.hash() {
-		validator.set_state(validation_state);
-		// The init_parachain_validator function clear the state every time,
-		// so we should write the state again.
-		LightClientStateSeal::<B, LightValidationState<B>>::seal_to_static_file(
-			validator.get_state(),
-		)?;
+	let init_state = if genesis_hash == params.genesis_header.hash() {
 		info!("Found already initialized light client with Genesis Hash: {:?}", genesis_hash);
-	}
+		validation_state
+	} else {
+		info!(
+			"Previous light client db belongs to another parentchain genesis. Creating new: {:?}",
+			genesis_hash
+		);
+		RelayState::new(params.genesis_header, vec![]).into()
+	};
+
+	let validator = init_parachain_validator::<B, OCallApi>(ocall_api, init_state)?;
 	info!("light client state: {:?}", validator);
+
+	LightClientStateSeal::<B, LightValidationState<B>>::seal_to_static_file(validator.get_state())?;
 	Ok(validator)
 }
 
-fn get_validation_state<B: Block>() -> Result<(LightValidationState<B>, B::Hash)>
-where
-	B: Block,
-{
+// Todo: Implement this on the `LightClientStateSeal` itself.
+fn get_validation_state<B: Block>() -> Result<(LightValidationState<B>, B::Hash)> {
 	let validation_state =
 		LightClientStateSeal::<B, LightValidationState<B>>::unseal_from_static_file()?;
 
-	let relay = validation_state
-		.tracked_relays
-		.get(&validation_state.num_relays)
-		.ok_or(Error::NoSuchRelayExists)?;
+	let relay = validation_state.get_relay();
 	let genesis_hash = relay.header_hashes[0];
 
 	Ok((validation_state, genesis_hash))
 }
 
 fn init_grandpa_validator<B, OCallApi>(
-	params: GrandpaParams<B::Header>,
 	ocall_api: Arc<OCallApi>,
+	state: LightValidationState<B>,
 ) -> Result<LightValidation<B, OCallApi>>
 where
 	B: Block,
 	NumberFor<B>: finality_grandpa::BlockNumberOps,
 	OCallApi: EnclaveOnChainOCallApi,
 {
 	let finality: Arc<Box<dyn Finality<B> + Sync + Send + 'static>> =
-		Arc::new(Box::new(GrandpaFinality {}));
-	let mut validator = LightValidation::<B, OCallApi>::new(ocall_api, finality);
-	validator.initialize_grandpa_relay(
-		params.genesis_header,
-		params.authorities,
-		params.authority_proof,
-	)?;
+		Arc::new(Box::new(GrandpaFinality));
+
+	let validator = LightValidation::<B, OCallApi>::new(ocall_api, finality, state);
 
-	LightClientStateSeal::<B, LightValidationState<B>>::seal_to_static_file(validator.get_state())?;
 	Ok(validator)
 }
 
 fn init_parachain_validator<B, OCallApi>(
-	params: SimpleParams<B::Header>,
 	ocall_api: Arc<OCallApi>,
+	state: LightValidationState<B>,
 ) -> Result<LightValidation<B, OCallApi>>
 where
 	B: Block,
 	NumberFor<B>: finality_grandpa::BlockNumberOps,
 	OCallApi: EnclaveOnChainOCallApi,
 {
 	let finality: Arc<Box<dyn Finality<B> + Sync + Send + 'static>> =
-		Arc::new(Box::new(ParachainFinality {}));
-	let mut validator = LightValidation::<B, OCallApi>::new(ocall_api, finality);
-	validator.initialize_parachain_relay(params.genesis_header, AuthorityList::default())?;
+		Arc::new(Box::new(ParachainFinality));
 
-	LightClientStateSeal::<B, LightValidationState<B>>::seal_to_static_file(validator.get_state())?;
+	let validator = LightValidation::<B, OCallApi>::new(ocall_api, finality, state);
 	Ok(validator)
 }

core/parentchain/light-client/src/lib.rs

@@ -32,7 +32,6 @@ pub use sp_finality_grandpa::{AuthorityList, SetId};
 
 use crate::light_validation_state::LightValidationState;
 use error::Error;
-use itp_storage::StorageProof;
 use sp_finality_grandpa::{AuthorityId, AuthorityWeight, ConsensusLog, GRANDPA_ENGINE_ID};
 use sp_runtime::{
 	generic::{Digest, OpaqueDigestItemId, SignedBlock},
@@ -73,28 +72,9 @@ pub trait Validator<Block: ParentchainBlockTrait>
 where
 	NumberFor<Block>: finality_grandpa::BlockNumberOps,
 {
-	fn initialize_grandpa_relay(
-		&mut self,
-		block_header: Block::Header,
-		validator_set: AuthorityList,
-		validator_set_proof: StorageProof,
-	) -> Result<RelayId, Error>;
+	fn submit_block(&mut self, signed_block: &SignedBlock<Block>) -> Result<(), Error>;
 
-	fn initialize_parachain_relay(
-		&mut self,
-		block_header: Block::Header,
-		validator_set: AuthorityList,
-	) -> Result<RelayId, Error>;
-
-	fn submit_block(
-		&mut self,
-		relay_id: RelayId,
-		signed_block: &SignedBlock<Block>,
-	) -> Result<(), Error>;
-
-	fn check_xt_inclusion(&mut self, relay_id: RelayId, block: &Block) -> Result<(), Error>;
-
-	fn set_state(&mut self, state: LightValidationState<Block>);
+	fn check_xt_inclusion(&mut self, block: &Block) -> Result<(), Error>;
 
 	fn get_state(&self) -> &LightValidationState<Block>;
 }
@@ -105,17 +85,14 @@ pub trait ExtrinsicSender {
 }
 
 pub trait LightClientState<Block: ParentchainBlockTrait> {
-	fn num_xt_to_be_included(&self, relay_id: RelayId) -> Result<usize, Error>;
+	fn num_xt_to_be_included(&self) -> Result<usize, Error>;
 
-	fn genesis_hash(&self, relay_id: RelayId) -> Result<HashFor<Block>, Error>;
+	fn genesis_hash(&self) -> Result<HashFor<Block>, Error>;
 
-	fn latest_finalized_header(&self, relay_id: RelayId) -> Result<Block::Header, Error>;
+	fn latest_finalized_header(&self) -> Result<Block::Header, Error>;
 
 	// Todo: Check if we still need this after #423
-	fn penultimate_finalized_block_header(&self, relay_id: RelayId)
-		-> Result<Block::Header, Error>;
-
-	fn num_relays(&self) -> RelayId;
+	fn penultimate_finalized_block_header(&self) -> Result<Block::Header, Error>;
 }
 
 pub fn grandpa_log<Block: ParentchainBlockTrait>(