Skip to content
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

Update call status to InSidechainBlock only after block import #676

Merged
merged 8 commits into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 0 additions & 4 deletions cli/demo_direct_call.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 0 additions & 14 deletions cli/demo_sidechain.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""

Comment on lines -84 to -87
Copy link
Contributor

Choose a reason for hiding this comment

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

So the client call (i.e. listener) blocks until the sidechain block is imported (meaning state is updated)? That ensures when we continue that a getter will return the correct state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, currently the cli waits until InSidechainBlock by default. Ideally, we will introduce the option to choose until which state we want to listen to, just like in the susbtrate-api-client. But that's an open issue: #207

echo "Get balance of Alice's incognito account (on worker 1)"
${CLIENTWORKER1} trusted balance ${ICGACCOUNTALICE} --mrenclave ${MRENCLAVE}
echo ""
Expand All @@ -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"
Expand Down Expand Up @@ -139,5 +127,3 @@ fi
echo ""

exit 0


Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
2 changes: 1 addition & 1 deletion enclave-runtime/src/test/mocks/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions enclave-runtime/src/test/sidechain_aura_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
23 changes: 7 additions & 16 deletions enclave-runtime/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
9 changes: 2 additions & 7 deletions enclave-runtime/src/top_pool_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
25 changes: 8 additions & 17 deletions sidechain/block-composer/src/block_composer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand All @@ -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,
Expand Down Expand Up @@ -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))
Expand Down
27 changes: 14 additions & 13 deletions sidechain/consensus/aura/src/block_importer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down Expand Up @@ -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)
Expand Down
22 changes: 16 additions & 6 deletions sidechain/top-pool-executor/src/call_operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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>
Expand All @@ -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();
Expand All @@ -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());
}
}
5 changes: 5 additions & 0 deletions sidechain/top-pool-executor/src/call_operator_mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

@haerdib haerdib Mar 14, 2022

Choose a reason for hiding this comment

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

I added a FIXME here, because I think adding unit test doesn't make sense right now.. in case we decide to update the top pool to call pool.

}
}
4 changes: 2 additions & 2 deletions sidechain/top-pool-executor/src/getter_operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions sidechain/top-pool-executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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:
Expand Down
Loading