Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
introduce is_syncing in sync_parentchain
  • Loading branch information
clangenb committed Nov 29, 2023
commit 3acdc2ebadafe1df58731f51f50dc0d39cc21d19
1 change: 1 addition & 0 deletions core-primitives/enclave-api/ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ extern "C" {
events_proofs_size: usize,
parentchain_id: *const u8,
parentchain_id_size: u32,
is_syncing: c_int,
) -> sgx_status_t;

pub fn set_nonce(
Expand Down
3 changes: 3 additions & 0 deletions core-primitives/enclave-api/src/sidechain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub trait Sidechain: Send + Sync + 'static {
events: &[Vec<u8>],
events_proofs: &[StorageProof],
parentchain_id: &ParentchainId,
is_syncing: bool,
) -> EnclaveResult<()>;

fn execute_trusted_calls(&self) -> EnclaveResult<()>;
Expand All @@ -56,6 +57,7 @@ mod impl_ffi {
events: &[Vec<u8>],
events_proofs: &[StorageProof],
parentchain_id: &ParentchainId,
is_syncing: bool,
) -> EnclaveResult<()> {
let mut retval = sgx_status_t::SGX_SUCCESS;
let blocks_enc = blocks.encode();
Expand All @@ -75,6 +77,7 @@ mod impl_ffi {
events_proofs_enc.len(),
parentchain_id_enc.as_ptr(),
parentchain_id_enc.len() as u32,
is_syncing.into(),
)
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,14 @@ impl<BlockImporter, SignedBlockType> DispatchBlockImport<SignedBlockType>
where
BlockImporter: ImportParentchainBlocks<SignedBlockType = SignedBlockType>,
{
fn dispatch_import(&self, blocks: Vec<SignedBlockType>, events: Vec<Vec<u8>>) -> Result<()> {
fn dispatch_import(
&self,
blocks: Vec<SignedBlockType>,
events: Vec<Vec<u8>>,
_is_syncing: bool,
) -> Result<()> {
// _is_syncing does not matter for the immediate dispatcher, behavoiur is the same. Immediate block import.

debug!("Importing {} parentchain blocks", blocks.len());
self.block_importer.import_parentchain_blocks(blocks, events)?;
debug!("Notifying {} observers of import", self.import_event_observers.len());
Expand Down
18 changes: 14 additions & 4 deletions core/parentchain/block-import-dispatcher/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,12 @@ pub trait DispatchBlockImport<SignedBlockType> {
/// Dispatch blocks to be imported.
///
/// The blocks may be imported immediately, get queued, delayed or grouped.
fn dispatch_import(&self, blocks: Vec<SignedBlockType>, events: Vec<Vec<u8>>) -> Result<()>;
fn dispatch_import(
&self,
blocks: Vec<SignedBlockType>,
events: Vec<Vec<u8>>,
is_syncing: bool,
) -> Result<()>;
}

/// Wrapper for the actual dispatchers. Allows to define one global type for
Expand Down Expand Up @@ -96,15 +101,20 @@ where
TriggeredDispatcher: DispatchBlockImport<SignedBlockType>,
ImmediateDispatcher: DispatchBlockImport<SignedBlockType>,
{
fn dispatch_import(&self, blocks: Vec<SignedBlockType>, events: Vec<Vec<u8>>) -> Result<()> {
fn dispatch_import(
&self,
blocks: Vec<SignedBlockType>,
events: Vec<Vec<u8>>,
is_syncing: bool,
) -> Result<()> {
match self {
BlockImportDispatcher::TriggeredDispatcher(dispatcher) => {
log::info!("TRIGGERED DISPATCHER MATCH");
dispatcher.dispatch_import(blocks, events)
dispatcher.dispatch_import(blocks, events, is_syncing)
},
BlockImportDispatcher::ImmediateDispatcher(dispatcher) => {
log::info!("IMMEDIATE DISPATCHER MATCH");
dispatcher.dispatch_import(blocks, events)
dispatcher.dispatch_import(blocks, events, is_syncing)
},
BlockImportDispatcher::EmptyDispatcher => {
log::info!("EMPTY DISPATCHER DISPATCHER MATCH");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,23 @@ where
&self,
blocks: Vec<SignedBlockType>,
events: Vec<RawEventsPerBlock>,
is_syncing: bool,
) -> Result<()> {
trace!(
"Pushing parentchain block(s) and event(s) ({}) ({}) to import queue",
"Triggered dispatcher received block(s) and event(s) ({}) ({})",
blocks.len(),
events.len()
);
// Push all the blocks to be dispatched into the queue.
self.events_queue.push_multiple(events).map_err(Error::ImportQueue)?;
self.import_queue.push_multiple(blocks).map_err(Error::ImportQueue)
if is_syncing {
trace!("Triggered is in sync mode, immediately importing blocks and events");
self.block_importer
.import_parentchain_blocks(blocks, events)
Copy link
Collaborator

@brenzi brenzi Dec 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this simple is_syncing bool flag is not enough

  • It needs to be clear to other validateers what parentchain block is the first one which must be considered for indirect invocation (in SCV and OCW mode). therefore, that "first relevant parentchain block should be stored in the STF state. Most likely, we should define this as the block when the primary worker enclave got registered for the first time, because the shard state will only be written to once the primary worker is in sync and starts to process parentchain blocks. caveat: if the shardid differs from mrenclave and the shardid gets registered a lot later, we will be importing too many blocks - but I don't think this is critical
  • callin it "is_syncing" is misleading. we're always syncing. I think it should be called "is_initial_sync" or similar
  • for the initial sync, the events and indirect invocations shouldn't even be searched for and they shouldn't be validated using proofs to accelerate the process

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with all points. I think there was a reason why it has been implemented the way it is now, but I forgot, and I don't see a reason why this is currently like that.

  • Substrate introduced the term is_major_sync, which is true whenever a node is far behind the best block. This concept does not really apply in our case, as we will send the light-client db to an outdated worker. However, what I am trying to say is that we stay somewhat in the substrate-jargon if we use is_initial_sync.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be more descriptive to call the flag ignore_invocations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, if this is the only thing that this flag is going to be used for yes, but I am not 100% if it is. Regardless, as long as we keep it like that, I agree. It could also be something like: verify_only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, we could also use this as some point maybe for non-authoring sidechain rpc-nodes, if we ever want to have them.

.map_err(|e| Error::BlockImport(e))
} else {
trace!("pushing blocks and events to import queues");
self.events_queue.push_multiple(events).map_err(Error::ImportQueue)?;
self.import_queue.push_multiple(blocks).map_err(Error::ImportQueue)
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion enclave-runtime/Enclave.edl
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ enclave {
[in, size=blocks_size] uint8_t* blocks, size_t blocks_size,
[in, size=events_size] uint8_t* events, size_t events_size,
[in, size=events_proofs_size] uint8_t* events_proofs, size_t events_proofs_size,
[in, size=parentchain_id_size] uint8_t* parentchain_id, uint32_t parentchain_id_size
[in, size=parentchain_id_size] uint8_t* parentchain_id, uint32_t parentchain_id_size,
int is_syncing
);

public sgx_status_t set_nonce(
Expand Down
42 changes: 36 additions & 6 deletions enclave-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use crate::{
},
};
use codec::Decode;
use core::ffi::c_int;
use itc_parentchain::{
block_import_dispatcher::{
triggered_dispatcher::TriggerParentchainBlockImport, DispatchBlockImport,
Expand Down Expand Up @@ -437,6 +438,7 @@ pub unsafe extern "C" fn sync_parentchain(
events_proofs_to_sync_size: usize,
parentchain_id: *const u8,
parentchain_id_size: u32,
is_syncing: c_int,
) -> sgx_status_t {
if let Err(e) = sync_parentchain_internal(
blocks_to_sync,
Expand All @@ -447,6 +449,7 @@ pub unsafe extern "C" fn sync_parentchain(
events_proofs_to_sync_size,
parentchain_id,
parentchain_id_size,
is_syncing == 1,
) {
error!("Error synching parentchain: {:?}", e);
}
Expand All @@ -464,6 +467,7 @@ unsafe fn sync_parentchain_internal(
events_proofs_to_sync_size: usize,
parentchain_id: *const u8,
parentchain_id_size: u32,
is_syncing: bool,
) -> Result<()> {
let blocks_to_sync = Vec::<SignedBlock>::decode_raw(blocks_to_sync, blocks_to_sync_size)?;
let events_proofs_to_sync =
Expand All @@ -483,6 +487,7 @@ unsafe fn sync_parentchain_internal(
blocks_to_sync,
events_to_sync,
&parentchain_id,
is_syncing,
)
}

Expand All @@ -498,6 +503,7 @@ fn dispatch_parentchain_blocks_for_import<WorkerModeProvider: ProvideWorkerMode>
blocks_to_sync: Vec<SignedBlock>,
events_to_sync: Vec<Vec<u8>>,
id: &ParentchainId,
is_syncing: bool,
) -> Result<()> {
if WorkerModeProvider::worker_mode() == WorkerMode::Teeracle {
trace!("Not importing any parentchain blocks");
Expand All @@ -507,27 +513,51 @@ fn dispatch_parentchain_blocks_for_import<WorkerModeProvider: ProvideWorkerMode>
match id {
ParentchainId::Integritee => {
if let Ok(handler) = GLOBAL_INTEGRITEE_SOLOCHAIN_HANDLER_COMPONENT.get() {
handler.import_dispatcher.dispatch_import(blocks_to_sync, events_to_sync)?;
handler.import_dispatcher.dispatch_import(
blocks_to_sync,
events_to_sync,
is_syncing,
)?;
} else if let Ok(handler) = GLOBAL_INTEGRITEE_PARACHAIN_HANDLER_COMPONENT.get() {
handler.import_dispatcher.dispatch_import(blocks_to_sync, events_to_sync)?;
handler.import_dispatcher.dispatch_import(
blocks_to_sync,
events_to_sync,
is_syncing,
)?;
} else {
return Err(Error::NoIntegriteeParentchainAssigned)
};
},
ParentchainId::TargetA => {
if let Ok(handler) = GLOBAL_TARGET_A_SOLOCHAIN_HANDLER_COMPONENT.get() {
handler.import_dispatcher.dispatch_import(blocks_to_sync, events_to_sync)?;
handler.import_dispatcher.dispatch_import(
blocks_to_sync,
events_to_sync,
is_syncing,
)?;
} else if let Ok(handler) = GLOBAL_TARGET_A_PARACHAIN_HANDLER_COMPONENT.get() {
handler.import_dispatcher.dispatch_import(blocks_to_sync, events_to_sync)?;
handler.import_dispatcher.dispatch_import(
blocks_to_sync,
events_to_sync,
is_syncing,
)?;
} else {
return Err(Error::NoTargetAParentchainAssigned)
};
},
ParentchainId::TargetB => {
if let Ok(handler) = GLOBAL_TARGET_B_SOLOCHAIN_HANDLER_COMPONENT.get() {
handler.import_dispatcher.dispatch_import(blocks_to_sync, events_to_sync)?;
handler.import_dispatcher.dispatch_import(
blocks_to_sync,
events_to_sync,
is_syncing,
)?;
} else if let Ok(handler) = GLOBAL_TARGET_B_PARACHAIN_HANDLER_COMPONENT.get() {
handler.import_dispatcher.dispatch_import(blocks_to_sync, events_to_sync)?;
handler.import_dispatcher.dispatch_import(
blocks_to_sync,
events_to_sync,
is_syncing,
)?;
} else {
return Err(Error::NoTargetBParentchainAssigned)
};
Expand Down
6 changes: 3 additions & 3 deletions service/src/main_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ fn start_worker<E, T, D, InitializationHandler, WorkerModeProvider>(

// Syncing all parentchain blocks, this might take a while..
let last_synced_header =
parentchain_handler.sync_parentchain(last_synced_header).unwrap();
parentchain_handler.sync_parentchain(last_synced_header, true).unwrap();

start_parentchain_header_subscription_thread(parentchain_handler, last_synced_header);
},
Expand Down Expand Up @@ -620,7 +620,7 @@ fn init_target_parentchain<E>(

// Syncing all parentchain blocks, this might take a while..
let last_synched_header =
parentchain_handler.sync_parentchain(last_synched_header).unwrap();
parentchain_handler.sync_parentchain(last_synched_header, true).unwrap();

start_parentchain_header_subscription_thread(parentchain_handler, last_synched_header)
}
Expand Down Expand Up @@ -1030,7 +1030,7 @@ fn subscribe_to_parentchain_new_headers<E: EnclaveBase + Sidechain>(
new_header.number
);

last_synced_header = parentchain_handler.sync_parentchain(last_synced_header)?;
last_synced_header = parentchain_handler.sync_parentchain(last_synced_header, false)?;
}
}

Expand Down
25 changes: 12 additions & 13 deletions service/src/parentchain_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ pub trait HandleParentchain {

/// Fetches the parentchain blocks to sync from the parentchain and feeds them to the enclave.
/// Returns the latest synced block header.
fn sync_parentchain(&self, last_synced_header: Header) -> ServiceResult<Header>;

/// Triggers the import of the synced parentchain blocks inside the enclave.
fn trigger_parentchain_block_import(&self) -> ServiceResult<()>;
fn sync_parentchain(
&self,
last_synced_header: Header,
is_syncing: bool,
) -> ServiceResult<Header>;

/// Syncs and directly imports parentchain blocks from the latest synced header
/// until the specified until_header.
Expand Down Expand Up @@ -140,7 +141,11 @@ where
.init_parentchain_components(self.parentchain_init_params.clone())?)
}

fn sync_parentchain(&self, last_synced_header: Header) -> ServiceResult<Header> {
fn sync_parentchain(
&self,
last_synced_header: Header,
is_syncing: bool,
) -> ServiceResult<Header> {
let id = self.parentchain_id();
trace!("[{:?}] Getting current head", id);
let curr_block = self
Expand Down Expand Up @@ -186,6 +191,7 @@ where
events_chunk_to_sync.as_slice(),
events_proofs_chunk_to_sync.as_slice(),
self.parentchain_id(),
is_syncing,
)?;

let api_client_until_synced_header = block_chunk_to_sync
Expand All @@ -204,11 +210,6 @@ where
}
}

fn trigger_parentchain_block_import(&self) -> ServiceResult<()> {
trace!("[{:?}] trigger parentchain block import", self.parentchain_id());
Ok(self.enclave_api.trigger_parentchain_block_import(self.parentchain_id())?)
}

fn sync_and_import_parentchain_until(
&self,
last_synced_header: &Header,
Expand All @@ -225,12 +226,10 @@ where
let mut last_synced_header = last_synced_header.clone();

while last_synced_header.number() < until_header.number() {
last_synced_header = self.sync_parentchain(last_synced_header)?;
last_synced_header = self.sync_parentchain(last_synced_header, true)?;
println!("[{:?}] synced block number: #{}", id, last_synced_header.number);
std::thread::sleep(std::time::Duration::from_secs(1));
}
self.trigger_parentchain_block_import()?;

Ok(last_synced_header)
}
}