-
Notifications
You must be signed in to change notification settings - Fork 46
Minor light-client refactor #1287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
83d3e48
47b6403
61dc3f7
1d0f150
06199f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
|
Comment on lines
-121
to
-122
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dito |
||
| let result = mutating_function(&mut light_validation_lock); | ||
| Seal::seal_to_static_file(light_validation_lock.get_state())?; | ||
| result | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,8 @@ use crate::{ | |
| error::Result, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here I mainly changed the initialization flow, which was really hard to handle because of side-effects of some functions. I think that there is still very much potential, but the code will be further amended in the course of #1242 anyhow. So I left it in the current state. |
||
| 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, | ||
| ¶ms.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. | ||
OverOrion marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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)?; | ||
clangenb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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>> | ||
|
Comment on lines
164
to
167
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function needed to be more lean, plus it had side effects (overwriting the disk with the freshly initialized light-client state, which was not always wished from on the calling site). |
||
| 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) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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>; | ||
|
Comment on lines
-76
to
-81
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No longer needed, we initialize the relay along with the constructor now - much leaner IMO. |
||
| 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>( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a global lock; The persisted light-client file will never contain more recent information than the already initialized validator. So we can skip reading from disk.