Update call status to InSidechainBlock only after block import (#676)

* rename on_block_created to on_block_imported

* update tx status only after block import

* fix clippy

* fix cargo test

* remove waiting time from demos

* readd removed comments

* udpate comments

* extract call removal logic from on_block_imported

cli/demo_direct_call.sh

@@ -84,10 +84,6 @@ echo "* Send ${AMOUNTTRANSFER} funds from Alice's incognito account to Bob's inc
 $CLIENT trusted transfer ${ICGACCOUNTALICE} ${ICGACCOUNTBOB} ${AMOUNTTRANSFER} --mrenclave ${MRENCLAVE} --direct
 echo ""
 
-echo "* Waiting 3 seconds"
-sleep 3
-echo ""
-
 echo "* Get balance of Alice's incognito account"
 RESULT=$(${CLIENT} trusted balance ${ICGACCOUNTALICE} --mrenclave ${MRENCLAVE} | xargs)
 echo $RESULT

cli/demo_sidechain.sh

@@ -81,10 +81,6 @@ echo "* Issue ${INITIALFUNDS} tokens to Alice's incognito account (on worker 1)"
 ${CLIENTWORKER1} trusted set-balance ${ICGACCOUNTALICE} ${INITIALFUNDS} --mrenclave ${MRENCLAVE} --direct
 echo ""
 
-echo "* Waiting 2 seconds"
-sleep 2
-echo ""
-
 echo "Get balance of Alice's incognito account (on worker 1)"
 ${CLIENTWORKER1} trusted balance ${ICGACCOUNTALICE} --mrenclave ${MRENCLAVE}
 echo ""
@@ -94,19 +90,11 @@ echo "* First transfer: Send ${AMOUNTTRANSFER} funds from Alice's incognito acco
 $CLIENTWORKER1 trusted transfer ${ICGACCOUNTALICE} ${ICGACCOUNTBOB} ${AMOUNTTRANSFER} --mrenclave ${MRENCLAVE} --direct
 echo ""
 
-echo "* Waiting 2 seconds"
-sleep 2
-echo ""
-
 # Send funds from Alice to Bobs account, on worker 2
 echo "* Second transfer: Send ${AMOUNTTRANSFER} funds from Alice's incognito account to Bob's incognito account (on worker 2)"
 $CLIENTWORKER2 trusted transfer ${ICGACCOUNTALICE} ${ICGACCOUNTBOB} ${AMOUNTTRANSFER} --mrenclave ${MRENCLAVE} --direct
 echo ""
 
-echo "* Waiting 2 seconds"
-sleep 2
-echo ""
-
 echo "* Get balance of Alice's incognito account (on worker 2)"
 ALICE_BALANCE=$(${CLIENTWORKER2} trusted balance ${ICGACCOUNTALICE} --mrenclave ${MRENCLAVE} | xargs)
 echo "$ALICE_BALANCE"
@@ -139,5 +127,3 @@ fi
 echo ""
 
 exit 0
-
-

core-primitives/test/src/mock/trusted_operation_pool_mock.rs

@@ -211,7 +211,7 @@ impl TrustedOperationPool for TrustedOperationPoolMock {
 		unimplemented!()
 	}
 
-	fn on_block_created(&self, _hashes: &[Self::Hash], _block_hash: SidechainBlockHash) {}
+	fn on_block_imported(&self, _hashes: &[Self::Hash], _block_hash: SidechainBlockHash) {}
 
 	fn rpc_send_state(&self, _hash: Self::Hash, _state_encoded: Vec<u8>) -> Result<(), Error> {
 		Ok(())

enclave-runtime/src/test/mocks/types.rs

@@ -73,7 +73,7 @@ pub type TestTopPoolExecutor =
 	TopPoolOperationHandler<ParentchainBlock, SignedSidechainBlock, TestRpcAuthor, TestStfExecutor>;
 
 pub type TestBlockComposer =
-	BlockComposer<ParentchainBlock, SignedSidechainBlock, TestSigner, TestStateKey, TestRpcAuthor>;
+	BlockComposer<ParentchainBlock, SignedSidechainBlock, TestSigner, TestStateKey>;
 
 pub type TestBlockImporter = BlockImporter<
 	TestSigner,

enclave-runtime/src/test/sidechain_aura_tests.rs

@@ -96,8 +96,7 @@ pub fn produce_sidechain_block_and_import_it() {
 		parentchain_block_import_trigger.clone(),
 		ocall_api.clone(),
 	));
-	let block_composer =
-		Arc::new(TestBlockComposer::new(signer.clone(), state_key, rpc_author.clone()));
+	let block_composer = Arc::new(TestBlockComposer::new(signer.clone(), state_key));
 	let proposer_environment =
 		ProposerFactory::new(top_pool_operation_handler, stf_executor.clone(), block_composer);
 	let extrinsics_factory = ExtrinsicsFactoryMock::default();

enclave-runtime/src/tests.rs

@@ -154,12 +154,9 @@ pub extern "C" fn test_main_entrance() -> size_t {
 
 fn test_compose_block_and_confirmation() {
 	// given
-	let (rpc_author, _, shard, _, _, state_handler) = test_setup();
-	let block_composer = BlockComposer::<Block, SignedBlock, _, _, _>::new(
-		test_account(),
-		state_key(),
-		rpc_author.clone(),
-	);
+	let (_, _, shard, _, _, state_handler) = test_setup();
+	let block_composer =
+		BlockComposer::<Block, SignedBlock, _, _>::new(test_account(), state_key());
 
 	let signed_top_hashes: Vec<H256> = vec![[94; 32].into(), [1; 32].into()].to_vec();
 
@@ -285,11 +282,8 @@ fn test_create_block_and_confirmation_works() {
 		rpc_author.clone(),
 		stf_executor.clone(),
 	);
-	let block_composer = BlockComposer::<Block, SignedBlock, _, _, _>::new(
-		test_account(),
-		state_key(),
-		rpc_author.clone(),
-	);
+	let block_composer =
+		BlockComposer::<Block, SignedBlock, _, _>::new(test_account(), state_key());
 
 	let sender = funded_pair();
 	let receiver = unfunded_public();
@@ -356,11 +350,8 @@ fn test_create_state_diff() {
 		rpc_author.clone(),
 		stf_executor.clone(),
 	);
-	let block_composer = BlockComposer::<Block, SignedBlock, _, _, _>::new(
-		test_account(),
-		state_key(),
-		rpc_author.clone(),
-	);
+	let block_composer =
+		BlockComposer::<Block, SignedBlock, _, _>::new(test_account(), state_key());
 
 	let sender = funded_pair();
 	let receiver = unfunded_public();

enclave-runtime/src/top_pool_execution.rs

@@ -18,7 +18,7 @@
 use crate::{
 	error::{Error, Result},
 	global_components::{
-		GLOBAL_PARENTCHAIN_IMPORT_DISPATCHER_COMPONENT, GLOBAL_RPC_AUTHOR_COMPONENT,
+		GLOBAL_PARENTCHAIN_IMPORT_DISPATCHER_COMPONENT,
 		GLOBAL_SIDECHAIN_IMPORT_QUEUE_WORKER_COMPONENT,
 	},
 	ocall::OcallApi,
@@ -181,12 +181,7 @@ fn execute_top_pool_trusted_calls_internal() -> Result<()> {
 			Error::ComponentNotInitialized
 		})?;
 
-	let rpc_author = GLOBAL_RPC_AUTHOR_COMPONENT.get().ok_or_else(|| {
-		error!("Failed to retrieve rpc author component. Maybe it's not initialized?");
-		Error::ComponentNotInitialized
-	})?;
-
-	let block_composer = Arc::new(BlockComposer::new(authority.clone(), state_key, rpc_author));
+	let block_composer = Arc::new(BlockComposer::new(authority.clone(), state_key));
 
 	match yield_next_slot(
 		slot_beginning_timestamp,

sidechain/block-composer/src/block_composer.rs

@@ -26,15 +26,14 @@ use its_primitives::traits::{
 	Block as SidechainBlockTrait, SignBlock, SignedBlock as SignedSidechainBlockTrait,
 };
 use its_state::{LastBlockExt, SidechainDB, SidechainState, SidechainSystemExt, StateHash};
-use its_top_pool_rpc_author::traits::{AuthorApi, OnBlockCreated, SendState};
 use log::*;
 use sgx_externalities::SgxExternalitiesTrait;
 use sp_core::Pair;
 use sp_runtime::{
 	traits::{Block as ParentchainBlockTrait, Header},
 	MultiSignature,
 };
-use std::{format, marker::PhantomData, sync::Arc, vec::Vec};
+use std::{format, marker::PhantomData, vec::Vec};
 
 /// Compose a sidechain block and corresponding confirmation extrinsic for the parentchain
 ///
@@ -52,47 +51,40 @@ pub trait ComposeBlockAndConfirmation<Externalities, ParentchainBlock: Parentcha
 }
 
 /// Block composer implementation for the sidechain
-pub struct BlockComposer<ParentchainBlock, SignedSidechainBlock, Signer, StateKey, RpcAuthor> {
+pub struct BlockComposer<ParentchainBlock, SignedSidechainBlock, Signer, StateKey> {
 	signer: Signer,
 	state_key: StateKey,
-	rpc_author: Arc<RpcAuthor>,
 	_phantom: PhantomData<(ParentchainBlock, SignedSidechainBlock)>,
 }
 
-impl<ParentchainBlock, SignedSidechainBlock, Signer, StateKey, RpcAuthor>
-	BlockComposer<ParentchainBlock, SignedSidechainBlock, Signer, StateKey, RpcAuthor>
+impl<ParentchainBlock, SignedSidechainBlock, Signer, StateKey>
+	BlockComposer<ParentchainBlock, SignedSidechainBlock, Signer, StateKey>
 where
 	ParentchainBlock: ParentchainBlockTrait<Hash = H256>,
 	SignedSidechainBlock:
 		SignedSidechainBlockTrait<Public = Signer::Public, Signature = MultiSignature>,
 	SignedSidechainBlock::Block:
 		SidechainBlockTrait<ShardIdentifier = H256, Public = sp_core::ed25519::Public>,
 	SignedSidechainBlock::Signature: From<Signer::Signature>,
-	RpcAuthor: AuthorApi<H256, ParentchainBlock::Hash>
-		+ OnBlockCreated<Hash = ParentchainBlock::Hash>
-		+ SendState<Hash = ParentchainBlock::Hash>,
 	Signer: Pair<Public = sp_core::ed25519::Public>,
 	Signer::Public: Encode,
 	StateKey: StateCrypto,
 {
-	pub fn new(signer: Signer, state_key: StateKey, rpc_author: Arc<RpcAuthor>) -> Self {
-		BlockComposer { signer, state_key, rpc_author, _phantom: Default::default() }
+	pub fn new(signer: Signer, state_key: StateKey) -> Self {
+		BlockComposer { signer, state_key, _phantom: Default::default() }
 	}
 }
 
-impl<ParentchainBlock, SignedSidechainBlock, Signer, StateKey, RpcAuthor, Externalities>
+impl<ParentchainBlock, SignedSidechainBlock, Signer, StateKey, Externalities>
 	ComposeBlockAndConfirmation<Externalities, ParentchainBlock>
-	for BlockComposer<ParentchainBlock, SignedSidechainBlock, Signer, StateKey, RpcAuthor>
+	for BlockComposer<ParentchainBlock, SignedSidechainBlock, Signer, StateKey>
 where
 	ParentchainBlock: ParentchainBlockTrait<Hash = H256>,
 	SignedSidechainBlock:
 		SignedSidechainBlockTrait<Public = Signer::Public, Signature = MultiSignature>,
 	SignedSidechainBlock::Block:
 		SidechainBlockTrait<ShardIdentifier = H256, Public = sp_core::ed25519::Public>,
 	SignedSidechainBlock::Signature: From<Signer::Signature>,
-	RpcAuthor: AuthorApi<H256, ParentchainBlock::Hash>
-		+ OnBlockCreated<Hash = ParentchainBlock::Hash>
-		+ SendState<Hash = ParentchainBlock::Hash>,
 	Externalities: SgxExternalitiesTrait + SidechainState + SidechainSystemExt + StateHash + Encode,
 	Signer: Pair<Public = sp_core::ed25519::Public>,
 	Signer::Public: Encode,
@@ -150,7 +142,6 @@ where
 
 		let opaque_call = create_proposed_sidechain_block_call(shard, block_hash);
 
-		self.rpc_author.on_block_created(block.signed_top_hashes(), block.hash());
 		let signed_block = block.sign_block(&self.signer);
 
 		Ok((opaque_call, signed_block))

sidechain/consensus/aura/src/block_importer.rs

@@ -132,21 +132,25 @@ impl<
 		}
 	}
 
-	pub(crate) fn remove_calls_from_top_pool(
-		&self,
-		signed_top_hashes: &[H256],
-		shard: &ShardIdentifierFor<SignedSidechainBlock>,
-	) {
-		let executed_operations = signed_top_hashes
+	pub(crate) fn update_top_pool(&self, sidechain_block: &SignedSidechainBlock::Block) {
+		// FIXME: we should take the rpc author here directly #547.
+
+		// Notify pool about imported block for status updates of the calls.
+		self.top_pool_executor.on_block_imported(sidechain_block);
+
+		// Remove calls from pool.
+		let executed_operations = sidechain_block
+			.signed_top_hashes()
 			.iter()
 			.map(|hash| {
 				// Only successfully executed operations are included in a block.
 				ExecutedOperation::success(*hash, TrustedOperationOrHash::Hash(*hash), Vec::new())
 			})
 			.collect();
-		// FIXME: we should take the rpc author here directly #547
-		let unremoved_calls =
-			self.top_pool_executor.remove_calls_from_pool(shard, executed_operations);
+
+		let unremoved_calls = self
+			.top_pool_executor
+			.remove_calls_from_pool(&sidechain_block.shard_id(), executed_operations);
 
 		for unremoved_call in unremoved_calls {
 			error!(
@@ -321,10 +325,7 @@ impl<
 		// If the block has been proposed by this enclave, remove all successfully applied
 		// trusted calls from the top pool.
 		if self.block_author_is_self(sidechain_block.block_author()) {
-			self.remove_calls_from_top_pool(
-				sidechain_block.signed_top_hashes(),
-				&sidechain_block.shard_id(),
-			)
+			self.update_top_pool(sidechain_block)
 		}
 
 		// Send metric about sidechain block height (i.e. block number)

sidechain/top-pool-executor/src/call_operator.rs

@@ -18,12 +18,12 @@
 use crate::{error::Result, TopPoolOperationHandler};
 use ita_stf::TrustedCallSigned;
 use itp_stf_executor::traits::{StateUpdateProposer, StfExecuteTimedGettersBatch};
-use itp_types::{ShardIdentifier, H256};
+use itp_types::H256;
 use its_primitives::traits::{
 	Block as SidechainBlockTrait, ShardIdentifierFor, SignedBlock as SignedSidechainBlockTrait,
 };
 use its_state::{SidechainState, SidechainSystemExt, StateHash};
-use its_top_pool_rpc_author::traits::{AuthorApi, OnBlockCreated, SendState};
+use its_top_pool_rpc_author::traits::{AuthorApi, OnBlockImported, SendState};
 use log::*;
 use sgx_externalities::SgxExternalitiesTrait;
 use sp_runtime::{traits::Block as ParentchainBlockTrait, MultiSignature};
@@ -49,8 +49,11 @@ pub trait TopPoolCallOperator<
 	fn remove_calls_from_pool(
 		&self,
 		shard: &ShardIdentifierFor<SignedSidechainBlock>,
-		calls: Vec<ExecutedOperation>,
+		executed_calls: Vec<ExecutedOperation>,
 	) -> Vec<ExecutedOperation>;
+
+	// Notify pool about block import for status updates
+	fn on_block_imported(&self, block: &SignedSidechainBlock::Block);
 }
 
 impl<ParentchainBlock, SignedSidechainBlock, RpcAuthor, StfExecutor>
@@ -63,19 +66,22 @@ where
 	SignedSidechainBlock::Block:
 		SidechainBlockTrait<ShardIdentifier = H256, Public = sp_core::ed25519::Public>,
 	RpcAuthor: AuthorApi<H256, ParentchainBlock::Hash>
-		+ OnBlockCreated<Hash = ParentchainBlock::Hash>
+		+ OnBlockImported<Hash = ParentchainBlock::Hash>
 		+ SendState<Hash = ParentchainBlock::Hash>,
 	StfExecutor: StateUpdateProposer + StfExecuteTimedGettersBatch,
 	<StfExecutor as StateUpdateProposer>::Externalities:
 		SgxExternalitiesTrait + SidechainState + SidechainSystemExt + StateHash,
 {
-	fn get_trusted_calls(&self, shard: &ShardIdentifier) -> Result<Vec<TrustedCallSigned>> {
+	fn get_trusted_calls(
+		&self,
+		shard: &ShardIdentifierFor<SignedSidechainBlock>,
+	) -> Result<Vec<TrustedCallSigned>> {
 		Ok(self.rpc_author.get_pending_tops_separated(*shard)?.0)
 	}
 
 	fn remove_calls_from_pool(
 		&self,
-		shard: &ShardIdentifier,
+		shard: &ShardIdentifierFor<SignedSidechainBlock>,
 		executed_calls: Vec<ExecutedOperation>,
 	) -> Vec<ExecutedOperation> {
 		let mut failed_to_remove = Vec::new();
@@ -93,4 +99,8 @@ where
 		}
 		failed_to_remove
 	}
+
+	fn on_block_imported(&self, block: &SignedSidechainBlock::Block) {
+		self.rpc_author.on_block_imported(block.signed_top_hashes(), block.hash());
+	}
 }

sidechain/top-pool-executor/src/call_operator_mock.rs

@@ -97,4 +97,9 @@ where
 		remove_call_invoked_lock.push((*shard, calls));
 		Default::default()
 	}
+
+	fn on_block_imported(&self, _block: &SignedSidechainBlock::Block) {
+		// Do nothing for now
+		// FIXME: We should include unit tests to see if pool is notified about block import
+	}
 }

sidechain/top-pool-executor/src/getter_operator.rs

@@ -27,7 +27,7 @@ use its_primitives::traits::{
 	Block as SidechainBlockTrait, SignedBlock as SignedSidechainBlockTrait,
 };
 use its_state::{SidechainState, SidechainSystemExt, StateHash};
-use its_top_pool_rpc_author::traits::{AuthorApi, OnBlockCreated, SendState};
+use its_top_pool_rpc_author::traits::{AuthorApi, OnBlockImported, SendState};
 use log::*;
 use sgx_externalities::SgxExternalitiesTrait;
 use sp_runtime::{traits::Block as ParentchainBlockTrait, MultiSignature};
@@ -64,7 +64,7 @@ where
 	SignedSidechainBlock::Block:
 		SidechainBlockTrait<ShardIdentifier = H256, Public = sp_core::ed25519::Public>,
 	RpcAuthor: AuthorApi<H256, ParentchainBlock::Hash>
-		+ OnBlockCreated<Hash = ParentchainBlock::Hash>
+		+ OnBlockImported<Hash = ParentchainBlock::Hash>
 		+ SendState<Hash = ParentchainBlock::Hash>,
 	StfExecutor: StateUpdateProposer + StfExecuteTimedGettersBatch,
 	<StfExecutor as StateUpdateProposer>::Externalities:

sidechain/top-pool-executor/src/lib.rs

@@ -54,7 +54,7 @@ use its_primitives::traits::{
 	Block as SidechainBlockTrait, SignedBlock as SignedSidechainBlockTrait,
 };
 use its_state::{SidechainState, SidechainSystemExt, StateHash};
-use its_top_pool_rpc_author::traits::{AuthorApi, OnBlockCreated, SendState};
+use its_top_pool_rpc_author::traits::{AuthorApi, OnBlockImported, SendState};
 use sgx_externalities::SgxExternalitiesTrait;
 use sp_runtime::{traits::Block as ParentchainBlockTrait, MultiSignature};
 use std::{marker::PhantomData, sync::Arc};
@@ -77,7 +77,7 @@ where
 	SignedSidechainBlock::Block:
 		SidechainBlockTrait<ShardIdentifier = H256, Public = sp_core::ed25519::Public>,
 	RpcAuthor: AuthorApi<H256, ParentchainBlock::Hash>
-		+ OnBlockCreated<Hash = ParentchainBlock::Hash>
+		+ OnBlockImported<Hash = ParentchainBlock::Hash>
 		+ SendState<Hash = ParentchainBlock::Hash>,
 	StfExecutor: StateUpdateProposer + StfExecuteTimedGettersBatch,
 	<StfExecutor as StateUpdateProposer>::Externalities: