Extract the STF execution logic to a separate crate 'stf-executor' #485

murerfel · 2021-11-02T12:38:50Z

A further refactoring step towards #301 . This time I extracted code from the enclave-runtime that is related to executing logic on the STF. This includes:

Trusted calls
Trusted getters
General state updates
Block updates

The main motivation for separating this code to a separate crate, is that the sidechain components use it, and it's the reason why we still have quite a bit of sidechain implementation in the enclave-runtime instead of the sidechain crate.

The extracted code has not been much beautified or re-designed, it's mostly moved in its original state. I mention this, because there are certainly quite a few improvements that could be done on those parts of the code (that now reside in stf-executor). But in order to move along and get to the end-goal (#301), I leave it as-is for now.

murerfel · 2021-11-02T12:39:26Z

Cargo.toml

@@ -16,6 +16,7 @@ members = [
    "core-primitives/settings",
    "core-primitives/sgx/crypto",
    "core-primitives/sgx/io",
+    "core-primitives/stf-executor",


new crate in core-primitives: stf-executor

murerfel · 2021-11-02T12:40:31Z

app-libs/stf/src/hash.rs

-//! Extrinsic helpers for author RPC module.
-
+use crate::TrustedOperation;
 use codec::{Decode, Encode};
-use ita_stf::TrustedOperation;
 use std::vec::Vec;

-/// RPC Trusted call or hash
+/// Trusted operation Or hash
 ///
 /// Allows to refer to trusted calls either by its raw representation or its hash.
-#[derive(Debug, Encode, Decode)]
+#[derive(Clone, Debug, Encode, Decode)]
 pub enum TrustedOperationOrHash<Hash> {
 	/// The hash of the call.
 	Hash(Hash),


Moved this from the top-pool-rpc-author crate to the ita-stf crate, so it can be re-used in our new crate stf-executor.

murerfel · 2021-11-02T12:41:35Z

app-libs/stf/src/lib.rs

+pub type StfResult<T> = Result<T, StfError>;
+
+#[derive(Debug, Display, PartialEq, Eq)]
+pub enum StfError {
+	#[display(fmt = "Insufficient privileges {:?}, are you sure you are root?", _0)]
+	MissingPrivileges(AccountId),
+	#[display(fmt = "Error dispatching runtime call. {:?}", _0)]
+	Dispatch(String),
+	#[display(fmt = "Not enough funds to perform operation")]
+	MissingFunds,
+	#[display(fmt = "Account does not exist {:?}", _0)]
+	InexistentAccount(AccountId),
+	#[display(fmt = "Invalid Nonce {:?}", _0)]
+	InvalidNonce(Index),
+	StorageHashMismatch,
+	InvalidStorageDiff,
+}


Moved the result and error type definition out of the sgx-only module, in order to have more parts of the code available in both std and sgx feature.

murerfel · 2021-11-02T12:42:53Z

app-libs/stf/src/stf_sgx.rs

-	pub fn get_state(ext: &mut State, getter: Getter) -> Option<Vec<u8>> {
+	pub fn get_state(ext: &mut impl SgxExternalitiesTrait, getter: Getter) -> Option<Vec<u8>> {


Generalize this method signature, making it consistent with some of the other methods that already used this type of signature.

murerfel · 2021-11-02T12:43:36Z

app-libs/stf/src/stf_sgx.rs

-				TrustedGetter::free_balance(who) =>
+				TrustedGetter::free_balance(who) => {


Seems like there was a change in the formatter after some update that happened on master? Now we have some formatting changes in this PR as a result. Either that, or this crate was never properly formatted to begin with (although shouldn't the formatter have complained?)

murerfel · 2021-11-02T12:47:31Z

core-primitives/ocall-api/src/lib.rs

+pub extern crate alloc;
+
+use alloc::vec::Vec;
 use codec::{Decode, Encode};
 use core::fmt::Debug;
 use itp_types::{TrustedOperationStatus, WorkerRequest, WorkerResponse};
 use its_primitives::traits::SignedBlock;
 use sgx_types::*;
 use sp_runtime::OpaqueExtrinsic;
-use sp_std::prelude::Vec;


Here I'm not sure: We have 3 ways to bring some of the alloc or std types into our sgx feature:

extern crate alloc + use alloc::vec::Vec

use std::vec::Vec

use sp_std::prelude::Vec (re-export in sp_std)

What do you guys think? Which option do you prefer and why?

I think it does not matter. extern crate alloc + use alloc::vec::Vec is the only one that would be available always. But as it is more verbose, I prefer this the least.

For crates that are close to supporting plain no_std, I would choose alloc still because:

sp_std: pulls in sp-io, which assumes wasm in no_std, implying that is does not support plain no_std. The library can be used no_std only in wasm (e.g. substrate-node-runtime`), or in sgx (where we patch sp-io, with sgx-sp-io)

sgx: can be used no_std only in an sgx-environment.

I'm against option three, because I don't think we should import substrate deps just to use vec::Vec.

Option 1 & 2.. I don't really care. You said that sgx ports Vec to alloc anyway, so it's only about style. Since we've been using std::vec::Vec quite often, maybe lets stick to that.

murerfel · 2021-11-02T12:50:52Z

core-primitives/stf-executor/src/executor.rs

+/// STF Executor implementation
+///
+///
+pub struct StfExecutor<OCallApi, StateHandler> {
+	ocall_api: Arc<OCallApi>,
+	state_handler: Arc<StateHandler>,
+}


This file now contains all the relevant implementation for executing calls, getters and operations in general on the STF. It's all moved from the enclave-runtime. It's probably a bit large and does not have any unit-tests 😞 . There are a couple of indirect tests of these parts, in the enclave-runtime.
Unfortunately this module is only available in sgx feature, because we heavily use Stf and all related structures.

So we should introduce unit tests for this part.. I guess? Maybe create an issue for this if its too much work for this PR?

Having unit tests would certainly be a great improvement. And we're lacking them everywhere. So I'm not sure an issue 'create unit tests for this' will do much good. In my experience those kind of issues never get prioritized in a sprint 😝 Next time we touch this code and we extend or change the behavior, we need to introduce unit tests. So writing the tests should be part of the next feature that happens in this part.

You got me there.. can't reason against that :(

Well you could argue to still do them now 😄 I'm a bit on the fence about it myself, because I like to write unit tests. And on the other hand, I should make some progress on the actual feature (#301) I'm trying to achieve

I also agree. Maybe add a //FIXME comment. Not an issue but atleast something that someone will stumble upon when this code part is touched?

murerfel · 2021-11-02T12:52:35Z

core-primitives/stf-executor/src/executor.rs

+	fn execute_trusted_call_on_stf<PB, E>(
+		&self,
+		state: &mut E,
+		stf_call_signed: &TrustedCallSigned,
+		header: &PB::Header,
+		shard: &ShardIdentifier,
+		post_processing: StatePostProcessing,
+	) -> Result<ExecutedOperation>


A re-usable method to execute a single trusted call on the STF (not part of a trait)

murerfel · 2021-11-02T12:55:00Z

core-primitives/stf-executor/src/executor.rs

+	fn execute_trusted_call<PB>(
+		&self,
+		calls: &mut Vec<OpaqueCall>,
+		stf_call_signed: &TrustedCallSigned,
+		header: &PB::Header,
+		shard: &ShardIdentifier,
+		post_processing: StatePostProcessing,
+	) -> Result<Option<(H256, H256)>>


I quite dislike the fact that the result of this method is both in the returned value, and in the &mut Vec<OpaqueCall> parameter. In some parts, I replaced this out parameter by just a return value. But not here yet.

Yes, this has been bugging me for ages too.

murerfel · 2021-11-02T12:56:21Z

core-primitives/stf-executor/src/executor.rs

+impl<OCallApi, StateHandler> StfExecuteShieldFunds for StfExecutor<OCallApi, StateHandler>
+where
+	OCallApi: EnclaveAttestationOCallApi + EnclaveOnChainOCallApi + GetStorageVerified,
+	StateHandler: HandleState,
+{
+	fn execute_shield_funds(
+		&self,
+		account: AccountId,
+		amount: Amount,
+		shard: &ShardIdentifier,
+		calls: &mut Vec<OpaqueCall>,
+	) -> Result<H256> {
+		let (state_lock, mut state) = self.state_handler.load_for_mutation(shard)?;
+
+		let root = Stf::get_root(&mut state);
+		let nonce = Stf::account_nonce(&mut state, &root);
+
+		let trusted_call = TrustedCallSigned::new(
+			TrustedCall::balance_shield(root, account, amount),
+			nonce,
+			Default::default(), //don't care about signature here
+		);
+
+		Stf::execute(&mut state, trusted_call, calls).map_err::<Error, _>(|e| e.into())?;
+
+		self.state_handler.write(state, state_lock, shard).map_err(|e| e.into())
+	}
+}


Execute shield funds logic on the Stf

murerfel · 2021-11-02T14:35:37Z

core-primitives/stf-executor/src/executor.rs

+	fn execute_timed_calls_batch<PB, F>(
+		&self,
+		trusted_calls: &[TrustedCallSigned],
+		header: &PB::Header,
+		shard: &ShardIdentifier,
+		max_exec_duration: Duration,
+		prepare_state_function: F,
+	) -> Result<ExecutionResult>
+	where
+		PB: BlockT<Hash = H256>,
+		F: FnOnce(Self::Externalities) -> Self::Externalities,
+	{


Execute a batch of trusted calls with a given time budget and return an ExecutionResult which includes the previous state hash, the execution status, and more information.

murerfel · 2021-11-02T14:37:19Z

core-primitives/stf-executor/src/lib.rs

+/// Result of an execution on the STF
+///
+/// Contains multiple executed calls
+#[derive(Clone, Debug)]
+pub struct ExecutionResult {
+	pub previous_state_hash: H256,
+	pub executed_operations: Vec<ExecutedOperation>,
+}


Utility structure for returning information about the execution of a collection of operations.

I really like the abstractions here. 👍

Very cool. I really like this approach. However, is the ExecutionResult naming correct? Until now, for me an execution was the execution of one extrsinic. Maybe I'm in the wrong here, but how about BatchExecutionResult?

I re-named it to BatchExecutionResult 👍

murerfel · 2021-11-02T14:40:22Z

enclave-runtime/src/lib.rs

-fn sync_blocks_on_light_client<PB, V, OCallApi, StateHandler>(
+fn sync_blocks_on_light_client<PB, V, OCallApi, StfExecutor>(
 	blocks_to_sync: Vec<SignedBlockG<PB>>,
 	validator: &mut V,
 	on_chain_ocall_api: &OCallApi,
-	state_handler: &StateHandler,
+	stf_executor: &StfExecutor,


In the enclave-runtime the StfExecutor now replaces the StateHandler in a lot of places (not everywhere) and thus hides the state handler details.

murerfel · 2021-11-02T14:42:17Z

enclave-runtime/src/lib.rs

+		|s| {
+			let mut sidechain_db = SidechainDB::<SB::Block, _>::new(s);
+			sidechain_db.set_block_number(&sidechain_db.get_block_number().map_or(1, |n| n + 1));
+			sidechain_db.set_timestamp(&now_as_u64());
+			sidechain_db.ext
+		},


The logic for setting block number, timestamp etc is now packed into a state 'pre-processing' lambda.

I think, this looks very nice.

haerdib

Complicated stuff.. left some comments.

haerdib · 2021-11-03T09:28:03Z

core-primitives/ocall-api/src/lib.rs

+pub extern crate alloc;
+
+use alloc::vec::Vec;
 use codec::{Decode, Encode};
 use core::fmt::Debug;
 use itp_types::{TrustedOperationStatus, WorkerRequest, WorkerResponse};
 use its_primitives::traits::SignedBlock;
 use sgx_types::*;
 use sp_runtime::OpaqueExtrinsic;
-use sp_std::prelude::Vec;


I'm against option three, because I don't think we should import substrate deps just to use vec::Vec.

Option 1 & 2.. I don't really care. You said that sgx ports Vec to alloc anyway, so it's only about style. Since we've been using std::vec::Vec quite often, maybe lets stick to that.

haerdib · 2021-11-03T09:33:46Z

core-primitives/stf-executor/src/error.rs

+	#[error("Trusted operation has invalid signature")]
+	OperationHasInvalidSignature,
+	#[error("SGX error, status: {0}")]
+	SgxError(sgx_status_t),


Not for this PR, but as a general Info in case you're introducing a new error type: Clippy will warn about using an Error ending in the future rust version: https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names

So don't name them xxxxError

I think you have a point here, the error enum element should not contain 'error' itself, I will fix that 👍

haerdib · 2021-11-03T09:56:44Z

core-primitives/stf-executor/src/executor.rs

+/// STF Executor implementation
+///
+///
+pub struct StfExecutor<OCallApi, StateHandler> {
+	ocall_api: Arc<OCallApi>,
+	state_handler: Arc<StateHandler>,
+}


So we should introduce unit tests for this part.. I guess? Maybe create an issue for this if its too much work for this PR?

haerdib · 2021-11-03T10:13:28Z

core-primitives/stf-executor/src/executor.rs

+
+use crate::{
+	error::{Error, Result},
+	traits::{


Is it really necessary to have so many traits? Is there no other way to abstract the stf?

This is the design question of how many methods should a trait have. In other languages, recommendations for interfaces are in the range of 2-5 or 2-7. With traits, where we have trait mix-in (with +) that makes trait aggregation very easy, we can achieve very fine granularity by having one method per trait.
In our case here, most consumers don't ever need more than 2 of these methods. So grouping them together in a trait, will cause unnecessary exposure of methods. That was my reasoning behind this design. However, I think this can be open for debate, I'd be happy to hear your arguments 😄

I second Felix. However, the real merit of this will not be obvious before we start having generic functions only requiring one of these 'subtraits'.

However, by better core design, we'd might want to some of these traits, but I think this is a different construction site.

We'd might want to do __ what with these traits? I think you lost the verb there in the sentence 😅

haha, thought faster than I typed. 😄 I meant 'remove'. However, not in terms of combining traits, but with a better core design. :P

Ah okay, now it makes sense 😝. Yes I totally agree, I think these stf executor traits can be reduced in number, because they are somewhat redundant and can be combined into more generalized trait methods. However, I also believe refactoring that requires a bit of effort, it's not that straight forward (and will require writing some tests in advance, to make sure we don't break stuff in the process).

My reasoning is just to keep it simple. The stf is made to be adaptable by clients such that they can easily integrate their stuff into the sgx-runtime. As long as we keep in mind the users who want to remove unshielding & shielding, or whatever, I'm fine with it. And it's not like I have a better solution here myself ;)

We can certainly make it more simple. What I have here in this PR is just extracting code as-is into a crate. It is by no means the end-result as we would want to have it. There are still many improvements that can be done on this part, this was just a first step.

haerdib · 2021-11-03T10:18:17Z

core-primitives/stf-executor/src/lib.rs

+/// Result of an execution on the STF
+///
+/// Contains multiple executed calls
+#[derive(Clone, Debug)]
+pub struct ExecutionResult {
+	pub previous_state_hash: H256,
+	pub executed_operations: Vec<ExecutedOperation>,
+}


Very cool. I really like this approach. However, is the ExecutionResult naming correct? Until now, for me an execution was the execution of one extrsinic. Maybe I'm in the wrong here, but how about BatchExecutionResult?

haerdib · 2021-11-03T10:20:53Z

core-primitives/stf-executor/src/lib.rs

+			.collect()
+	}
+
+	pub fn get_all_execution_hashes(&self) -> Vec<ExecutionHashes> {


Renaming suggestion: get_executed_operation_hashes

I can do that 👍

haerdib · 2021-11-03T10:24:14Z

core-primitives/types/src/lib.rs

@@ -64,7 +65,7 @@ pub type MrEnclave = [u8; 32];
 pub type BlockHash = H256;

 pub type ConfirmCallFn = ([u8; 2], ShardIdentifier, H256, Vec<u8>);
-pub type ShieldFundsFn = ([u8; 2], Vec<u8>, u128, ShardIdentifier);
+pub type ShieldFundsFn = ([u8; 2], Vec<u8>, Amount, ShardIdentifier);


haerdib · 2021-11-03T10:29:14Z

core-primitives/stf-executor/src/lib.rs

+
+/// Hash results of an operation execution on the STF
+#[derive(Clone, Debug)]
+pub struct ExecutionHashes {


For me this naming was a little confusing. ExecutionHashes could also be the State hashes after execution. We could simply name it ExtrinsicHash to follow substrates example. Or OperationHash. ( I guess it should be singular, as it only contains one hash each? Also, why are both, call hashes and operation hashes needed?

Yes the naming is not ideal here. It's probably a combination of me not understanding exactly what it represents, and the thing having a confusing name before the refactoring.
I don't know why we have both a OperationHash and a CallHash - this is just ported over as it was before. I also don't exactly understand what the difference should be. ExecutionHashes should probably be replaced by a better name. Provided we still need OperationHash AND CallHash, what could the aggregation of those two be called?

I think @haerdib introduced these (not sure), so she might correct my statements.

I have taken a look at the code. Deriving from the current usage of handle_trusted_worker_call, which is only at two places. The first time, we only care about the potential error, the second time, we are only interested in the trusted_operation_hash, which is put into a call_hash vector (!!!), I sense some code smell here. :)

I am confident, we can get rid of one. I actually think we should not use the trusted operations hash. Reasoning: this is an internal helper struct of the worker. The client sends either TrustedCalls or TrustedGetters AFAIK. If we want to give a block inclusion proof to a client in the future, it will be more straightforward if we hash the exact object the client has sent to the worker. The hash is not needed for anything else.

I would simply use the following structure:

ExecutionResult { // hash of the executed trusted call. call_hash: H256 }

I'd be happy to get rid of one of those hashes. And yes, we mix the names in the process, that adds to the confusion. We now use the operation_hash and use it in a call_hashes vector. And you suggest to only use the call_hash and get rid of the operation_hash? Won't that break something, because we're now returning a different hash value?

I think that's fine if everything is still working. But we might want to create an issue to discuss the use case of TrustedOperation itself @clangenb ?

Yes, I think @mullefel change is fine.

I also agree to discuess the usage of trusted op.

fyi: I had to revert back to using the operation hash (not the call hash), since we have tests that check for that. So to be on the safe side, I'll use the operation hash for now to not alter any behavior in the code.

Ok, even better. 👍

haerdib · 2021-11-03T11:16:01Z

enclave-runtime/src/lib.rs

+				shard,
+				executed_operation.is_success(),
+			)
+			.unwrap();


I know this was here before, but I think we should not panic in case we can not remove a top. Should we?

We probably shouldn't. I'll change that to properly handle the result 👍

murerfel · 2021-11-03T15:30:01Z

Unfortunately I cannot answer to your suggestion: #485 (comment) , so I'll do it here.
BatchExecutionResult might already be an improvement, I'll re-name it. Maybe at some point we can come up with better names for these structures (provided they are still around after more refactoring).

further step towards moving out the sidechain logic out of the enclave-runtime

clangenb

In general, very impressive. I think you made a good compromise between simple modularization and refactoring.

Only some small questions.

clangenb · 2021-11-03T05:28:38Z

core-primitives/ocall-api/src/lib.rs

+pub extern crate alloc;
+
+use alloc::vec::Vec;
 use codec::{Decode, Encode};
 use core::fmt::Debug;
 use itp_types::{TrustedOperationStatus, WorkerRequest, WorkerResponse};
 use its_primitives::traits::SignedBlock;
 use sgx_types::*;
 use sp_runtime::OpaqueExtrinsic;
-use sp_std::prelude::Vec;


I think it does not matter. extern crate alloc + use alloc::vec::Vec is the only one that would be available always. But as it is more verbose, I prefer this the least.

For crates that are close to supporting plain no_std, I would choose alloc still because:

sp_std: pulls in sp-io, which assumes wasm in no_std, implying that is does not support plain no_std. The library can be used no_std only in wasm (e.g. substrate-node-runtime`), or in sgx (where we patch sp-io, with sgx-sp-io)

sgx: can be used no_std only in an sgx-environment.

clangenb · 2021-11-03T05:30:56Z

core-primitives/stf-executor/src/executor.rs

+	fn execute_trusted_call<PB>(
+		&self,
+		calls: &mut Vec<OpaqueCall>,
+		stf_call_signed: &TrustedCallSigned,
+		header: &PB::Header,
+		shard: &ShardIdentifier,
+		post_processing: StatePostProcessing,
+	) -> Result<Option<(H256, H256)>>


Yes, this has been bugging me for ages too.

sidechain/top-pool-rpc-author/src/author.rs

clangenb · 2021-11-03T06:03:51Z

core-primitives/stf-executor/src/lib.rs

+/// Result of an execution on the STF
+///
+/// Contains multiple executed calls
+#[derive(Clone, Debug)]
+pub struct ExecutionResult {
+	pub previous_state_hash: H256,
+	pub executed_operations: Vec<ExecutedOperation>,
+}


I really like the abstractions here. 👍

clangenb · 2021-11-03T06:36:04Z

core-primitives/stf-executor/src/executor.rs

+	}
+}
+
+impl<OCallApi, StateHandler> StfExecuteGenericUpdate for StfExecutor<OCallApi, StateHandler>


I think this can be even more useful, if we add Externalities to the generic arguments and require the SgxExternalities trait bound here; what do you think?

This would require the that the externalities are part of the StfExecutor as a PhantomData field, but this is totally fine.

Yes I can do that, it would further decouple us from the concrete SgxExternalities 👍

I made that change now. It required a bit of refactoring in the state handler as well, to make that trait generic over the externalities. And I also had to further generalize some of the method signature in the Stf struct. The explains the rather large change set that I had to commit on top of this PR 😅

Haha, yes, but I think it was worth the effort. Thanks! Looks good.

clangenb · 2021-11-04T03:45:45Z

core-primitives/stf-executor/src/executor.rs

+
+use crate::{
+	error::{Error, Result},
+	traits::{


However, by better core design, we'd might want to some of these traits, but I think this is a different construction site.

clangenb · 2021-11-04T04:19:09Z

core-primitives/stf-executor/src/lib.rs

+
+/// Hash results of an operation execution on the STF
+#[derive(Clone, Debug)]
+pub struct ExecutionHashes {


I think @haerdib introduced these (not sure), so she might correct my statements.

I have taken a look at the code. Deriving from the current usage of handle_trusted_worker_call, which is only at two places. The first time, we only care about the potential error, the second time, we are only interested in the trusted_operation_hash, which is put into a call_hash vector (!!!), I sense some code smell here. :)

I am confident, we can get rid of one. I actually think we should not use the trusted operations hash. Reasoning: this is an internal helper struct of the worker. The client sends either TrustedCalls or TrustedGetters AFAIK. If we want to give a block inclusion proof to a client in the future, it will be more straightforward if we hash the exact object the client has sent to the worker. The hash is not needed for anything else.

clangenb · 2021-11-04T04:20:38Z

core-primitives/stf-executor/src/lib.rs

+
+/// Hash results of an operation execution on the STF
+#[derive(Clone, Debug)]
+pub struct ExecutionHashes {


I would simply use the following structure:

ExecutionResult { // hash of the executed trusted call. call_hash: H256 }

clangenb · 2021-11-04T04:25:56Z

enclave-runtime/src/lib.rs

+		|s| {
+			let mut sidechain_db = SidechainDB::<SB::Block, _>::new(s);
+			sidechain_db.set_block_number(&sidechain_db.get_block_number().map_or(1, |n| n + 1));
+			sidechain_db.set_timestamp(&now_as_u64());
+			sidechain_db.ext
+		},


I think, this looks very nice.

clangenb · 2021-11-04T04:32:11Z

core-primitives/stf-executor/Cargo.toml

+[features]
+default = ["std"]
+std = [
+    "ita-stf/std",


This sucks big time, but I think we will have to live with this until we abstract TrustedCalls, TrustedOp, TrustedGetter into core-primitives.

We have the ita-stf dependency everywhere because of it, yes. I've looked at it a couple of times, and I'm not sure there is a 'simple' solution, provided we don't break with @brenzi 's intention to keep all the domain specific stuff in that crate (and TrustedCall, TrustedGetter etc. contain exactly the domain specific logic)

return only the call hash, not the operation hash

… executor

clangenb

Looks very good!

haerdib

Very cool, thanks !

murerfel commented Nov 2, 2021

View reviewed changes

murerfel self-assigned this Nov 2, 2021

murerfel force-pushed the feature/fm-trusted-call-handler branch from f55dcdd to 174fc87 Compare November 2, 2021 14:53

murerfel requested review from clangenb and haerdib November 2, 2021 15:46

murerfel marked this pull request as ready for review November 2, 2021 15:46

murerfel mentioned this pull request Nov 3, 2021

Separate send xts and send blocks #487

Merged

1 task

haerdib reviewed Nov 3, 2021

View reviewed changes

Felix Müller added 6 commits November 3, 2021 16:33

Extract stf executor crate from enclave-runtime

98011ea

introduce stf execution trait for timed batches

a93eaa7

Move move STF execution logic to the separate crate

bee6c1a

further step towards moving out the sidechain logic out of the enclave-runtime

Move trusted getter stf execution to the new stf-executor crate

fc25bc8

fix cargo fmt

7f1d740

Fixes and renamings according to PR discussions

12743a4

murerfel force-pushed the feature/fm-trusted-call-handler branch from 174fc87 to 12743a4 Compare November 3, 2021 15:33

clangenb reviewed Nov 4, 2021

View reviewed changes

Felix Müller added 3 commits November 4, 2021 08:23

Make the state handler and stf executor generic over Externalities

b4beb5e

Remove operation_hash from stf executor result

6daecdd

return only the call hash, not the operation hash

add fixme for unit tests in executor

1fa61ae

haerdib mentioned this pull request Nov 4, 2021

TrustedOperation wrapper still needed? #489

Closed

revert back to using the operation hash in the returned result of the…

0712f5b

… executor

clangenb approved these changes Nov 4, 2021

View reviewed changes

haerdib approved these changes Nov 4, 2021

View reviewed changes

murerfel merged commit 4378e63 into master Nov 4, 2021

murerfel deleted the feature/fm-trusted-call-handler branch November 4, 2021 10:30

		pub fn get_state(ext: &mut State, getter: Getter) -> Option<Vec<u8>> {
		pub fn get_state(ext: &mut impl SgxExternalitiesTrait, getter: Getter) -> Option<Vec<u8>> {

		TrustedGetter::free_balance(who) =>
		TrustedGetter::free_balance(who) => {

Extract the STF execution logic to a separate crate 'stf-executor' #485

Extract the STF execution logic to a separate crate 'stf-executor' #485

Conversation

murerfel commented Nov 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murerfel Nov 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murerfel Nov 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murerfel Nov 4, 2021 • edited Loading

Choose a reason for hiding this comment

haerdib Nov 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murerfel Nov 2, 2021 • edited Loading

Choose a reason for hiding this comment

murerfel Nov 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murerfel Nov 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haerdib left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murerfel Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murerfel Nov 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murerfel commented Nov 3, 2021 • edited Loading

clangenb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murerfel commented Nov 2, 2021 •

edited

Loading

murerfel Nov 2, 2021 •

edited

Loading

murerfel Nov 2, 2021 •

edited

Loading

murerfel Nov 4, 2021 •

edited

Loading

haerdib Nov 4, 2021 •

edited

Loading

murerfel Nov 2, 2021 •

edited

Loading

murerfel Nov 2, 2021 •

edited

Loading

murerfel Nov 2, 2021 •

edited

Loading

murerfel Nov 3, 2021 •

edited

Loading

murerfel Nov 4, 2021 •

edited

Loading

murerfel commented Nov 3, 2021 •

edited

Loading