Skip to content

Conversation

@murerfel
Copy link
Contributor

This implements a sub-task of #567 , where we have a new mechanism to suspend block production while we sync blocks from a peer. Also the coordinating component (called PeerBlockSync) is added in a first draft version (not integrated or in use yet), to illustrate how the block import command will be handled in the future. This is an additional layer around the regular BlockImporter, that

  1. Detects ancestry errors upon import and starts the process of fetching sidechain blocks from a peer
  2. Checks if block production has been suspended and if so, does not import a block (in the future we will want to cache them)
  3. Does the regular block import, just as before this change

The whole block production suspension and peer block syncing is not integrated and connected to anything yet, this will be done in a future PR.

Activity diagram for the peer block syncing:

peer_sync_activity

@murerfel murerfel self-assigned this Jan 12, 2022
Copy link
Contributor Author

@murerfel murerfel left a comment

Choose a reason for hiding this comment

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

Added my own review documentation comments

Comment on lines -232 to -248
fn handle_import_error(
&self,
signed_sidechain_block: &SignedSidechainBlock,
error: ConsensusError,
) -> Result<(), ConsensusError> {
match error {
ConsensusError::BlockAncestryMismatch(_last_imported_number, ref msg) => {
//FIXME: We should trigger block production suspension (TBD) here.
warn!("Could not import block {:?} due to: {:?}.", signed_sidechain_block, msg);
},
// Some erros, such as invalid author, should be ignored because we do not want
// to provide possible attack vectors.
_ => warn!("Ignoring import error of block {:?}.", signed_sidechain_block),
}

Err(error)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this part again, handling the import error is done in the 'parent' of the block importer. Otherwise the block importer would need to call itself, which makes this struct even more complex and is not good design IMO.

Comment on lines 29 to 41
/// Trait to suspend the production of sidechain blocks.
pub trait SuspendBlockProduction {
/// Suspend any sidechain block production.
fn suspend(&self) -> Result<()>;

/// Resume block sidechain block production.
fn resume(&self) -> Result<()>;
}

/// Trait to query if sidechain block production is suspended.
pub trait IsBlockProductionSuspended {
fn is_suspended(&self) -> Result<bool>;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simple traits for setting and querying the block production suspension state.

Comment on lines 43 to 47
/// Implementation for suspending and resuming sidechain block production.
#[derive(Default)]
pub struct BlockProductionSuspender {
is_suspended: RwLock<bool>,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The implementation is basically just a wrapper around a bool with a lock.

Comment on lines 59 to +58
#[cfg(test)]
mod mock;

#[cfg(test)]
mod block_importer_tests;
mod test;
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 re-arranged the test code in this crate, put everything in the test module, which in turn contains fixtures and mocks.

Comment on lines +18 to +38
pub mod types;

use itp_types::{AccountId, Enclave, Header};
use sp_runtime::traits::Header as HeaderTrait;
use std::time::Duration;

pub const SLOT_DURATION: Duration = Duration::from_millis(300);

pub fn validateer(account: AccountId) -> Enclave {
Enclave::new(account, Default::default(), Default::default(), Default::default())
}

pub fn default_header() -> Header {
Header::new(
Default::default(),
Default::default(),
Default::default(),
Default::default(),
Default::default(),
)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code extracted from mock.rs, unchanged.

limitations under the License.
*/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extracted from mock.rs, just re-arranged.

Comment on lines 31 to 36
/// Block importer mock.
pub struct BlockImportMock<ParentchainBlock, SignedSidechainBlock> {
import_result: Option<Result<()>>,
imported_blocks: RwLock<Vec<SignedSidechainBlock>>,
_phantom: PhantomData<(ParentchainBlock, SignedSidechainBlock)>,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Block importer mock that allows to set the result that should be returned from the block import and stores any blocks that were sent to be imported.

limitations under the License.
*/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also extracted from mock.rs

Comment on lines 144 to +151
ConsensusError::BlockAncestryMismatch(
last_block.block_number(),
"Invalid block number".into()
last_block.hash(),
format!(
"Invalid block number, {} does not succeed {}",
block.block_number(),
last_block.block_number()
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The BlockAncestryMismatch now also contains the hash of the last known sidechain block. This is needed for fetching sidechain blocks from a peer.

@murerfel murerfel changed the title Peer sync and block production suspension for sidechain [Sidechain] Peer sync and block production suspension Jan 13, 2022
@murerfel murerfel requested a review from haerdib January 13, 2022 15:14
Copy link
Contributor

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Looks very nice and clean. The implementation does make sense to me.

Only very few minor comments.

@murerfel murerfel requested review from clangenb and haerdib January 17, 2022 09:38
Copy link
Contributor

@haerdib haerdib left a comment

Choose a reason for hiding this comment

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

LGTM, thanks :)

@murerfel murerfel force-pushed the feature/fm-suspend-block-production branch from 56f5d08 to a41b84d Compare January 17, 2022 12:05
@murerfel murerfel force-pushed the feature/fm-suspend-block-production branch from a41b84d to dbac4e4 Compare January 18, 2022 06:38
@murerfel murerfel merged commit ec1dfb8 into master Jan 18, 2022
@murerfel murerfel deleted the feature/fm-suspend-block-production branch January 18, 2022 08:39
haerdib added a commit to ajuna-network/worker that referenced this pull request Feb 10, 2022
* Make mu-ra and untrusted worker url queriable (integritee-network#595)

* extract request_keys() to separate file

* remove providr input, add dummy getter function

* add node_api worker_for_shard call

* fix error message

* add primitives cache and rpc call

* fix tests

* add primitives-cache to workspace

* fix unit tests

* remove obsolete .yml provider from request-keys cmd

* remove provider_addr from CI py scripts

* fix reported worker address

* improve usability of rpc-client

* make it work

* fix rebase error

* add some delay

* update local setup script

* remove ugly async worker url, replace with enclave getter function

* some steps towards a working exmaple..

* add peer_updater

* fix unit test

* fix some test clippy warnings

* fix function name

* fix client mu ra url

* fix comment

* fix comment

* rename state_sync to appropriate request keys

* fix comments and add missing _size to untrusted_worker_addr

* update cargo.lock after rebase

* fix typos

* rename store_peers to set_peers

* fix comment

* move set_primitives to primitves cache repository

* return read guard instead of primittves clone

* rename config worker_rpc_port to trusted_worker_port

* remove obsolete Error enum from request_keys.rs

* fix unit tests

* move thread spawning back into watch fn

* rename worker-rpc-port to trusted-worker-port

* readd external worker address

* fix unit tests

* fix unit test

* add external addresses, optional port input and unit tests

* update test names

* [cli.yml] update shorts

* fix local setup configs

* change untrusted worker port to w

* [sidechain] detect out of sync error (integritee-network#606)

* inital commit

* remove unwrap_err from assert_matches

* Update substrate sp-core to version 4.1.0-dev (integritee-network#612)

Co-authored-by: Gaudenz Kessler <[email protected]>

* Renaming of unspecific SB and PB variable names (integritee-network#605)

* some clean up & add hanlde import error

* remove logic changes

* fix unit tests

* [aura block importer] rename SidechainBlock to SignedSidechainBlock

* fix rebase errors

* [aura mock] rename xxT import to xxTrait

* [aura verifier] rename SidechainBlock to SignedSidechainblock where appropriate

* fix rebase errors

* [aura] rename Sidechainblock to SignedSidechainBlock

* rename SB & PB to full written version and adapt to SignedSidechainBlock Where necessary

* [sidechain block imported] remove extra generic from SignedParentchainBlock

* some further SB & PB clean up

* rename B & SB to SidechainBlock & SignedSidechainBlock

* some further renaming

* completely remove SB

* rename all left over PBs

* remove rebase error & rename to SignedSidechainBlock

* rename to SignedSidechainBlock

* Sidechain peer fetch blocks - RPC client/server (integritee-network#580)

* WIP: RPC call to fetch sidechain blocks

* WIP: sidechain peer fetch crate with RPC server and client impl

* WIP: test for RPC peer sidechain block fetching

* fix unit test

* remove obsolete comment

* fix rebase error

* cargo fmt

* fix tests

* fix Fixme + add som Send+Sync to errors

* update add_block_to_batch to return error. Otherwise silent fail

* small comment fixes

* make some comments better understandable

* remove FIXME comment

* remove new lines

* fix trailing comments

* [peer-fetch] change order or crates in .toml

* [sidechain storage] fix error message of HeaderAncestryMismatch

* [sidechain storage] exchange match statement with ok_or

* [sidechain storage] use temp-dir in tests

* [sidechain storage] remove extra genesis block check

* fix rebase errors

* remove untrstued url, replace with untrusted peer fetcher

* [sidechain storage] fix comment

* update delete_last_block description comment

* [sidechain storage] fix comment grammer

* move FetchUntrustedPeers trait to the top

* [FetchBlocksFromPeer] extend description comment

* update cargo.lock

* rename get_blocks_following to get_blocks_after

* rename get_blocks_following to get_blocks_after

* rename all leftover "blocks_following" to "block_after"

Co-authored-by: Bigna Härdi <[email protected]>

* [Sidechain] Peer block fetching o-call implementation (integritee-network#619)

* introduce o-call for fetching sidechain blocks from peer
* re-name api-client-extensions to node-api-extensions

Sub-task of integritee-network#567

* add direct call rpc doc (integritee-network#620)

* add some doc

* add some structure to the links

* restructure rpc interface

* Update docs/README.md

Co-authored-by: gaudenzkessler <[email protected]>

* adapt readmes according to review comments

Co-authored-by: gaudenzkessler <[email protected]>

* [Sidechain] Peer sync and block production suspension (integritee-network#618)

Peer syncer implementation (not in use yet) and block production suspender (also not in use yet)

* update to most recent teaclave commit (integritee-network#624)

* Sync state from fellow validateer (integritee-network#615)

* rename request_keys to sync_state

* rename request_key_prov to request_state_prov

* rename request_keys.rs to sync_state.rs

* restructure key and state provisioning server

* some refactoring

* add TlsServer struct

* add test file

* rename key_provision_server to state_provisioning_server

* add unit test

* update unit test

* introduce mockable key handler struct

* shielding key success

* remove clippy warnings

* fix test

* add unit tests for KeyHandler

* rename to prepare for state inclusion

* rename seal_handler

* add shard as argument to sync state

* some more renaming

* add shard read & write process

* [SealHandler] add unit tests & fix state

* update networking test to include state

* add default shard

* add some documentation

* remove ugly for loop

* move authentications to separate file

* update comment

* remove obsolete, never ending loop

* add error logs

* remove extra phantom field

* add sgx feature flag

* remove global variables from test

* add join handle to test

* add some more logging info

* Change tokio runtime to use 2 worker threads. Gossiping spawns new tokio tasks. (integritee-network#626)

* Add state update sequence (integritee-network#632)

* add bock_import_sequence.svg

* move block_import.svg to docs/diagramms

* update diagramm

* add block import sequence

* RPC call to get metadata from sgx-runtime (integritee-network#642)

* RPC call to get metadata from sgx-runtime
- rcp call
- print sgx metadata cli

* Change from review: Metadata is already encoded

* Change from review

Co-authored-by: echevrier <[email protected]>

* bump substrate to commit 59649dd (integritee-network#645)

* update .tomls to new substrate versions

* cargo update

* RawEvent -> Event

* remove default from Accountid

* RawEvent -> Event

* cargo update enclave-runtime

* fix bump errors

* remove unused patch

* finaly compiling

* update sgx-runtime and substrate-api-client to github

* remove integritee-node-runtime patch

* cargo update -p std-std --precise 59649dd

* update Github Actions integritee node

* remove bh-config

* fix clippy

* fix cargo test

* update spec version

* update substrate-api-client

* update sgx-runtime source

* update substrate

* adjust node version values

* detect new game

* solve merge conflicts

* update sgx-runtime

* fix some things

* cargo format

Co-authored-by: gaudenzkessler <[email protected]>
Co-authored-by: Gaudenz Kessler <[email protected]>
Co-authored-by: Felix Müller <[email protected]>
Co-authored-by: echevrier <[email protected]>
Co-authored-by: echevrier <[email protected]>
haerdib added a commit to ajuna-network/worker that referenced this pull request Feb 10, 2022
* Make mu-ra and untrusted worker url queriable (integritee-network#595)

* extract request_keys() to separate file

* remove providr input, add dummy getter function

* add node_api worker_for_shard call

* fix error message

* add primitives cache and rpc call

* fix tests

* add primitives-cache to workspace

* fix unit tests

* remove obsolete .yml provider from request-keys cmd

* remove provider_addr from CI py scripts

* fix reported worker address

* improve usability of rpc-client

* make it work

* fix rebase error

* add some delay

* update local setup script

* remove ugly async worker url, replace with enclave getter function

* some steps towards a working exmaple..

* add peer_updater

* fix unit test

* fix some test clippy warnings

* fix function name

* fix client mu ra url

* fix comment

* fix comment

* rename state_sync to appropriate request keys

* fix comments and add missing _size to untrusted_worker_addr

* update cargo.lock after rebase

* fix typos

* rename store_peers to set_peers

* fix comment

* move set_primitives to primitves cache repository

* return read guard instead of primittves clone

* rename config worker_rpc_port to trusted_worker_port

* remove obsolete Error enum from request_keys.rs

* fix unit tests

* move thread spawning back into watch fn

* rename worker-rpc-port to trusted-worker-port

* readd external worker address

* fix unit tests

* fix unit test

* add external addresses, optional port input and unit tests

* update test names

* [cli.yml] update shorts

* fix local setup configs

* change untrusted worker port to w

* [sidechain] detect out of sync error (integritee-network#606)

* inital commit

* remove unwrap_err from assert_matches

* Update substrate sp-core to version 4.1.0-dev (integritee-network#612)

Co-authored-by: Gaudenz Kessler <[email protected]>

* Renaming of unspecific SB and PB variable names (integritee-network#605)

* some clean up & add hanlde import error

* remove logic changes

* fix unit tests

* [aura block importer] rename SidechainBlock to SignedSidechainBlock

* fix rebase errors

* [aura mock] rename xxT import to xxTrait

* [aura verifier] rename SidechainBlock to SignedSidechainblock where appropriate

* fix rebase errors

* [aura] rename Sidechainblock to SignedSidechainBlock

* rename SB & PB to full written version and adapt to SignedSidechainBlock Where necessary

* [sidechain block imported] remove extra generic from SignedParentchainBlock

* some further SB & PB clean up

* rename B & SB to SidechainBlock & SignedSidechainBlock

* some further renaming

* completely remove SB

* rename all left over PBs

* remove rebase error & rename to SignedSidechainBlock

* rename to SignedSidechainBlock

* Sidechain peer fetch blocks - RPC client/server (integritee-network#580)

* WIP: RPC call to fetch sidechain blocks

* WIP: sidechain peer fetch crate with RPC server and client impl

* WIP: test for RPC peer sidechain block fetching

* fix unit test

* remove obsolete comment

* fix rebase error

* cargo fmt

* fix tests

* fix Fixme + add som Send+Sync to errors

* update add_block_to_batch to return error. Otherwise silent fail

* small comment fixes

* make some comments better understandable

* remove FIXME comment

* remove new lines

* fix trailing comments

* [peer-fetch] change order or crates in .toml

* [sidechain storage] fix error message of HeaderAncestryMismatch

* [sidechain storage] exchange match statement with ok_or

* [sidechain storage] use temp-dir in tests

* [sidechain storage] remove extra genesis block check

* fix rebase errors

* remove untrstued url, replace with untrusted peer fetcher

* [sidechain storage] fix comment

* update delete_last_block description comment

* [sidechain storage] fix comment grammer

* move FetchUntrustedPeers trait to the top

* [FetchBlocksFromPeer] extend description comment

* update cargo.lock

* rename get_blocks_following to get_blocks_after

* rename get_blocks_following to get_blocks_after

* rename all leftover "blocks_following" to "block_after"

Co-authored-by: Bigna Härdi <[email protected]>

* [Sidechain] Peer block fetching o-call implementation (integritee-network#619)

* introduce o-call for fetching sidechain blocks from peer
* re-name api-client-extensions to node-api-extensions

Sub-task of integritee-network#567

* add direct call rpc doc (integritee-network#620)

* add some doc

* add some structure to the links

* restructure rpc interface

* Update docs/README.md

Co-authored-by: gaudenzkessler <[email protected]>

* adapt readmes according to review comments

Co-authored-by: gaudenzkessler <[email protected]>

* [Sidechain] Peer sync and block production suspension (integritee-network#618)

Peer syncer implementation (not in use yet) and block production suspender (also not in use yet)

* update to most recent teaclave commit (integritee-network#624)

* Sync state from fellow validateer (integritee-network#615)

* rename request_keys to sync_state

* rename request_key_prov to request_state_prov

* rename request_keys.rs to sync_state.rs

* restructure key and state provisioning server

* some refactoring

* add TlsServer struct

* add test file

* rename key_provision_server to state_provisioning_server

* add unit test

* update unit test

* introduce mockable key handler struct

* shielding key success

* remove clippy warnings

* fix test

* add unit tests for KeyHandler

* rename to prepare for state inclusion

* rename seal_handler

* add shard as argument to sync state

* some more renaming

* add shard read & write process

* [SealHandler] add unit tests & fix state

* update networking test to include state

* add default shard

* add some documentation

* remove ugly for loop

* move authentications to separate file

* update comment

* remove obsolete, never ending loop

* add error logs

* remove extra phantom field

* add sgx feature flag

* remove global variables from test

* add join handle to test

* add some more logging info

* Change tokio runtime to use 2 worker threads. Gossiping spawns new tokio tasks. (integritee-network#626)

* Add state update sequence (integritee-network#632)

* add bock_import_sequence.svg

* move block_import.svg to docs/diagramms

* update diagramm

* add block import sequence

* RPC call to get metadata from sgx-runtime (integritee-network#642)

* RPC call to get metadata from sgx-runtime
- rcp call
- print sgx metadata cli

* Change from review: Metadata is already encoded

* Change from review

Co-authored-by: echevrier <[email protected]>

* bump substrate to commit 59649dd (integritee-network#645)

* update .tomls to new substrate versions

* cargo update

* RawEvent -> Event

* remove default from Accountid

* RawEvent -> Event

* cargo update enclave-runtime

* fix bump errors

* remove unused patch

* finaly compiling

* update sgx-runtime and substrate-api-client to github

* remove integritee-node-runtime patch

* cargo update -p std-std --precise 59649dd

* update Github Actions integritee node

* remove bh-config

* fix clippy

* fix cargo test

* update spec version

* update substrate-api-client

* update sgx-runtime source

* update substrate

* adjust node version values

* detect new game

* solve merge conflicts

* update sgx-runtime

* fix some things

* cargo format

Co-authored-by: haerdib <[email protected]>
Co-authored-by: Gaudenz Kessler <[email protected]>
Co-authored-by: Felix Müller <[email protected]>
Co-authored-by: Bigna Härdi <[email protected]>
Co-authored-by: echevrier <[email protected]>
Co-authored-by: echevrier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants