Sync state from fellow validateer by haerdib · Pull Request #615 · integritee-network/worker

haerdib · 2022-01-11T14:49:07Z

Divides tls_ra.rs into server and client
Adds header with opcode and payload length indicator, such that the ordering and length of the payload may vary.
add state syncing
add some unit tests

TODO:

sync state: add state handler
add unit tests

Tested manually with following steps (wasn't sure how to do an integration test here..)

start integritee node
start worker 1 with ./integritee-service -P 2094 -p 9994 -r 3494 run --dev
run demo_direct_call: ./demo_direct_call.sh -P 2094 -p 9994:
request-state with worker2 (with empty shard directory): ./integritee-service -p 9994 request-state
Stop worker 1 (such that no sidechain blocks will reach worker 2)
Start worker 2 ./integritee-service -P 2095 -p 9994 -r 3494 run --dev
Query balance of //AliceIncognito of second worker:

./integritee-cli -p 9994 -P 2095 trusted balance //AliceIncognito --mrenclave DBQbPN2WKh54fucAmpU7Z39ip5FYRBzzmnkJPmuQ1GNH
10000000000
./integritee-cli -p 9994 -P 2095 trusted balance //BobIncognito --mrenclave DBQbPN2WKh54fucAmpU7Z39ip5FYRBzzmnkJPmuQ1GNH
40000000000

closes #614

haerdib · 2022-01-19T09:05:33Z

enclave-runtime/src/tls_ra/authentication.rs

+
+*/
+
+//! Remote attestation certificate authentication of server and client


I did not change this part, simply copy pasted from the old tls_ra.rs file. So I'm unsure why there are so many unused variables in the function definition. Hence, I left as it is for now.

haerdib · 2022-01-19T09:12:18Z

enclave-runtime/src/tls_ra/tls_ra_client.rs

+}
+
+#[no_mangle]
+pub unsafe extern "C" fn request_state_provisioning(


Code from here on (in this file) left as is (copy pasted from tls_ra.rs), except the inclusion of the request_state_provisioning_internal for easy testing & usage of ?

haerdib · 2022-01-19T09:13:11Z

enclave-runtime/src/tls_ra/tls_ra_server.rs

+}
+
+#[no_mangle]
+pub unsafe extern "C" fn run_state_provisioning_server(


Code from here on (in this file) left as is (copy pasted from tls_ra.rs), except the inclusion of the run_state_provisioning_server_internal for easy testing & usage of ?

murerfel

All in all it's looking good - there are some issues I would like to have addressed ☺️

core-primitives/sgx/crypto/src/lib.rs

murerfel · 2022-01-19T08:56:31Z

enclave-runtime/src/tls_ra/mocks.rs

+	fn seal_shielding_key(&self, bytes: &[u8]) -> EnclaveResult<()> {
+		*SHIELDING_KEY.write().unwrap() = bytes.to_vec();
+		Ok(())
+	}


Why do we need these global variables in this mock? Is it not enough to use the members?

The problem lies with the mutability of the variables. We don't pass a mutable self here because it's not needed by the original function.

And if we have a mutable self, the whole request_state_provisioning_internal needs a mut reference input for the client_seal_handler, otherwise my server - client connection test would not work.

I didn't think adapting the code to use mutable references, which are not actually needed, are the way to go. Or what do you think?

What you need then, is RwLock on your members (that's something you'll see in many of my mock implementations). But they'll still be members and scoped inside the struct, not globally.

Better now? :)

Yes indeed, that's what I imagined ☺️

enclave-runtime/src/tls_ra/seal_handler.rs

murerfel · 2022-01-19T09:02:50Z

enclave-runtime/src/tls_ra/tests.rs

+	thread::spawn(move || {
+		run_state_provisioning_server(server_seal_handler);
+	});


You spawn a thread here, but you never join - so this thread keeps on running until the entire program is terminated? However, the thread should probably terminate when the test does?

Ops, that is correct. But joining simply waits for the thread to finish, it does not terminate anything. So in the worst case, the test will never end. I removed the (now obsolete) loop, so the thread finishes as soon it's done writing.

Yes simple joining wouldn't have solved problem if the thread is in an endless loop. You would've needed a sender/receiver to terminate the thread. But in case the loop is not required anymore, you can do the simple join.

Added a join 👍

enclave-runtime/src/tls_ra/tests.rs

enclave-runtime/src/tls_ra/authentication.rs

enclave-runtime/src/tls_ra/tls_ra_client.rs

enclave-runtime/src/tls_ra/tls_ra_server.rs

enclave-runtime/src/tls_ra/tls_ra_client.rs

echevrier

I am not a specialist in this part of the code, so my opinion is limited. It looks good to me. I really like the clean code and the tests

haerdib · 2022-01-19T13:45:35Z

I am not a specialist in this part of the code, so my opinion is limited.

And that's the reason why you're a good reviewer. If you don't understand something, it means it's not good code or not enough documented. Thanks for your comment ! 👍

murerfel · 2022-01-24T06:55:38Z

core-primitives/sgx/crypto/src/lib.rs

+#[cfg(all(feature = "mocks", feature = "sgx"))]
+pub mod mocks;


murerfel · 2022-01-24T07:00:13Z

enclave-runtime/src/tls_ra/mocks.rs

+	pub shielding_key: Arc<RwLock<Vec<u8>>>,
+	pub signing_key: Arc<RwLock<Vec<u8>>>,
+	pub state: Arc<RwLock<Vec<u8>>>,


I'm guessing you're using Arcs here for the #[derive(Clone)], because RwLock is not clonable? For the mock it's probably okay, but in a real implementation, this would most likely not be what we want. We'd have to implement clone ourselves and clone the inner value of RwLock.

Agreed, but for the mock that was the easiest way to implement a test. At least for what I could see.

murerfel · 2022-01-24T07:03:12Z

enclave-runtime/src/tls_ra/tests.rs

+	// Ensure server thread has finished.
+	server_thread_handle.join().unwrap();


murerfel · 2022-01-24T07:04:24Z

enclave-runtime/src/tls_ra/mocks.rs

+	fn seal_shielding_key(&self, bytes: &[u8]) -> EnclaveResult<()> {
+		*SHIELDING_KEY.write().unwrap() = bytes.to_vec();
+		Ok(())
+	}


Yes indeed, that's what I imagined ☺️

murerfel

It's looking good to me now 👍 good job

* 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 <gaudenz.kessler@scs.ch> * 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 <bhaerdi@devsgx02.scs-ad.scs.ch> * [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 <92718752+gaudenzkessler@users.noreply.github.com> * adapt readmes according to review comments Co-authored-by: gaudenzkessler <92718752+gaudenzkessler@users.noreply.github.com> * [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 <edith.chevrier@scs.ch> * 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 <92718752+gaudenzkessler@users.noreply.github.com> Co-authored-by: Gaudenz Kessler <gaudenz.kessler@scs.ch> Co-authored-by: Felix Müller <mullefel@users.noreply.github.com> Co-authored-by: echevrier <84318241+echevrier@users.noreply.github.com> Co-authored-by: echevrier <edith.chevrier@scs.ch>

* 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 <gaudenz.kessler@scs.ch> * 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 <bhaerdi@devsgx02.scs-ad.scs.ch> * [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 <92718752+gaudenzkessler@users.noreply.github.com> * adapt readmes according to review comments Co-authored-by: gaudenzkessler <92718752+gaudenzkessler@users.noreply.github.com> * [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 <edith.chevrier@scs.ch> * 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 <73821294+haerdib@users.noreply.github.com> Co-authored-by: Gaudenz Kessler <gaudenz.kessler@scs.ch> Co-authored-by: Felix Müller <mullefel@users.noreply.github.com> Co-authored-by: Bigna Härdi <bhaerdi@devsgx02.scs-ad.scs.ch> Co-authored-by: echevrier <84318241+echevrier@users.noreply.github.com> Co-authored-by: echevrier <edith.chevrier@scs.ch>

haerdib self-assigned this Jan 14, 2022

haerdib force-pushed the bh-sync-state-from-fellow-validateer branch from 8f7b908 to 3574be9 Compare January 17, 2022 16:18

haerdib requested a review from echevrier January 19, 2022 08:26

haerdib marked this pull request as ready for review January 19, 2022 08:27

haerdib requested a review from clangenb January 19, 2022 08:35

haerdib force-pushed the bh-sync-state-from-fellow-validateer branch from 9eebd29 to 277b49d Compare January 19, 2022 08:53

haerdib commented Jan 19, 2022

View reviewed changes

haerdib requested review from murerfel and removed request for clangenb January 19, 2022 09:08

haerdib commented Jan 19, 2022

View reviewed changes

murerfel reviewed Jan 19, 2022

View reviewed changes

echevrier approved these changes Jan 19, 2022

View reviewed changes

haerdib added 17 commits January 20, 2022 17:44

rename request_keys to sync_state

5e52beb

rename request_key_prov to request_state_prov

0d275c9

rename request_keys.rs to sync_state.rs

0ae2893

restructure key and state provisioning server

dc40dd0

some refactoring

fc3e318

add TlsServer struct

d5b5f16

add test file

14b24fe

rename key_provision_server to state_provisioning_server

4470b4e

add unit test

688214d

update unit test

7389461

introduce mockable key handler struct

37b454f

shielding key success

ada4ab8

remove clippy warnings

385864d

fix test

f725377

add unit tests for KeyHandler

5a006e6

rename to prepare for state inclusion

3be9d6a

rename seal_handler

1fb37a3

haerdib added 14 commits January 20, 2022 17:45

add shard as argument to sync state

e29edf1

some more renaming

09f7934

add shard read & write process

e90cf68

[SealHandler] add unit tests & fix state

e2e684f

update networking test to include state

93fde21

add default shard

659e83c

add some documentation

adaa20c

remove ugly for loop

2f5be11

move authentications to separate file

80f8061

update comment

f8dd5ca

remove obsolete, never ending loop

928d516

add error logs

92290db

remove extra phantom field

4e996d9

add sgx feature flag

5536dd1

haerdib force-pushed the bh-sync-state-from-fellow-validateer branch from 1041451 to 5536dd1 Compare January 20, 2022 16:47

haerdib added 3 commits January 20, 2022 18:05

remove global variables from test

10fc847

add join handle to test

a8d31fc

add some more logging info

b7f1e97

haerdib requested a review from murerfel January 21, 2022 08:24

murerfel reviewed Jan 24, 2022

View reviewed changes

murerfel approved these changes Jan 24, 2022

View reviewed changes

haerdib merged commit 9a14d49 into master Jan 24, 2022

haerdib deleted the bh-sync-state-from-fellow-validateer branch January 24, 2022 07:10


		*/

		//! Remote attestation certificate authentication of server and client

		#[cfg(all(feature = "mocks", feature = "sgx"))]
		pub mod mocks;

		// Ensure server thread has finished.
		server_thread_handle.join().unwrap();

Conversation

haerdib commented Jan 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

murerfel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

echevrier left a comment

Choose a reason for hiding this comment

Uh oh!

haerdib commented Jan 19, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

murerfel left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

haerdib commented Jan 11, 2022 •

edited

Loading