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.

Sign up for GitHub

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

Jump to bottom

multiple parentchain handlers #1420

Merged
merged 100 commits into from
Sep 8, 2023
Merged

multiple parentchain handlers #1420

merged 100 commits into from
Sep 8, 2023

Conversation

clangenb
Copy link
Contributor

@clangenb clangenb commented Aug 17, 2023 •
edited
Loading

Add support to connect to multiple parentchains, sending extrinsics to them and also interpret indirect calls from them.

Follow-ups

Working local setup running in the integritee-dev:0.2.1 container

  • build with: SGX_MODE=SW ADDITIONAL_FEATURES=dcap WORKER_MODE=offchain-worker make
  • run in terminal 1 from directory root: ./local-setup/launch.py ./local-setup/config/two-nodes-one-worker.json
  • open next terminal and tail logs with: cd local-setup && ./tmux_logger_two_nodes.sh
  • open next terminal and execute minimal demo script: cd cli && ./demo_shield_on_second_node_with_transfer_to_alice.sh

@clangenb clangenb added A5-teeracle A0-core Affects a core part A4-offchain B1-releasenotes C1-low 📌 Does not elevate a release containing this beyond "low priority" E3-hardmerge PR introduces a lot changes in a lot of files. Requires some effort to merge or rebase to. labels Aug 17, 2023
@clangenb clangenb marked this pull request as draft August 17, 2023 15:30
@clangenb
[enclave-runtime/initialization] lift some code out of the innermost …
c2663d1 
…function
@clangenb
[enclave-runtime/initialization] rename EnclaveIndirectExecutor to …
b866e92 
…`TeerexParentchainIndirectExecutor`
@clangenb
[enclave-runtime/initialization] rename stuff that is only for the pr…
2de2e2a 
…imary parentchain containing the teerex pallet
@clangenb
[enclave-runtime/initialization] naively add second set of components…
4b9b51b 
… needed for a second parachain
@clangenb
[enclave-runtime/initialization] introduce second nonce cache for the…
0f10edc 
… second parachain
@clangenb
[enclave-runtime/initialization/global_components] fix wrong type arg
a678568
@clangenb
[indirect-calls-executor] fix DenyAll implementation
f8a0b3e
@clangenb
[enclave-runtime/initialization] code compiles for instantiating a se…
1cb32dc 
…condary parachain parentchain handler.
@clangenb
[enclave-runtime/initialization] fmt
7e2dd91
@clangenb
[enclave-runtime/initialization] add second set of solochain parentch…
4cdc78c 
…ain handler
@clangenb
[enclave-runtime/initialization] extend parentchain init params with …
2d45de6 
…the secondary parentchains
@clangenb
[enclave-runtime/initialization] add second light-client seal
2b3e2b9
@clangenb
change parentchain init params enum, and add full code for initializa…
ba29650 
…tion.
@clangenb clangenb force-pushed the cl/multiple-parentchain-handlers branch from 199bca1 to ba29650 Compare August 17, 2023 15:31
@clangenb clangenb marked this pull request as ready for review September 7, 2023 14:31
clangenb
clangenb commented Sep 7, 2023
View reviewed changes
core-primitives/enclave-api/src/sidechain.rs
@@ -33,7 +34,7 @@ pub trait Sidechain: Send + Sync + 'static {
blocks: &[SignedBlock<ParentchainBlock>],
events: &[Vec<u8>],
events_proofs: &[StorageProof],
nonce: u32,
Copy link
Contributor Author

@clangenb clangenb Sep 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This was an argument, which was already unused, but we didn't bother to change the FFI interface as it seems.

core-primitives/stf-executor/src/executor.rs Show resolved Hide resolved
core/parentchain/block-import-dispatcher/src/lib.rs Show resolved Hide resolved
core/parentchain/block-importer/src/block_importer.rs Show resolved Hide resolved
core/parentchain/indirect-calls-executor/src/filter_metadata.rs Show resolved Hide resolved
enclave-runtime/src/initialization/parentchain/common.rs Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A lot of duplicate code here, but the globals prevent generics here, and given the priorities this should be fixed another time, and it might actually be good to finish the secondary light client first to see the full picture of re-used code.

enclave-runtime/src/initialization/parentchain/target_a_parachain.rs
Comment on lines +105 to +106
WorkerMode::Sidechain =>
unimplemented!("Can't run target a chain in sidechain mode yet."),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Notabene: We need some additional synchronization issues before we can use it in sidechain mode, like:

When do we import a secondary sidechain block? Also only the block author?

I haven't really thought about that yet, I decided to focus on the easy case for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nopt sure if I understand. I don't think we need a secondary sidechain. It is not the idea that multiple sidechains will operate on top of multiple L1's. It should be a single sidechain on top of the TargetA, but using the sidechain pallet and teerex on Integritee

enclave-runtime/src/lib.rs Show resolved Hide resolved
enclave-runtime/src/initialization/parentchain/mod.rs Show resolved Hide resolved
core/parentchain/indirect-calls-executor/src/indirect_calls/transfer_to_alice_shields_funds.rs Show resolved Hide resolved
@clangenb clangenb requested a review from brenzi September 7, 2023 15:21
@clangenb
[enclave-runtime/global-components] fix misallocation of `IntegriteeB…
272f233 
…lockImporter` to a Target A parentchain type. (this did not have any effect in the offchain-worker case)
brenzi
brenzi approved these changes Sep 8, 2023
View reviewed changes
Copy link
Collaborator

@brenzi brenzi left a comment

Choose a reason for hiding this comment

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

I didn't check details, but LGTM
Looking forward to applying this

@brenzi
Copy link
Collaborator

brenzi commented Sep 8, 2023

I reproduced the demo successfully. will merge

@brenzi brenzi merged commit 1323415 into master Sep 8, 2023
19 of 21 checks passed
@brenzi brenzi mentioned this pull request Sep 8, 2023
Merged
@clangenb clangenb deleted the cl/multiple-parentchain-handlers branch September 10, 2023 11:41
This was referenced Sep 12, 2023
Closed
Closed
Closed
Closed
Closed
Closed
clangenb added a commit to olisystems/BEST-Energy that referenced this pull request Sep 12, 2023
@clangenb
multiple parentchain handlers (integritee-network#1420)
c184277 
* [enclave-runtime/initialization] lift some code out of the innermost function

* [enclave-runtime/initialization] rename `EnclaveIndirectExecutor` to `TeerexParentchainIndirectExecutor`

* [enclave-runtime/initialization] rename stuff that is only for the primary parentchain containing the teerex pallet

* [enclave-runtime/initialization] naively add second set of components needed for a second parachain

* [enclave-runtime/initialization] introduce second nonce cache for the second parachain

* [enclave-runtime/initialization/global_components] fix wrong type arg

* [indirect-calls-executor] fix `DenyAll` implementation

* [enclave-runtime/initialization] code compiles for instantiating a secondary parachain parentchain handler.

* [enclave-runtime/initialization] fmt

* [enclave-runtime/initialization] add second set of solochain parentchain handler

* [enclave-runtime/initialization] extend parentchain init params with the secondary parentchains

* [enclave-runtime/initialization] add second light-client seal

* change parentchain init params enum, and add full code for initialization.

* fix some comments and naming

* [service] add cli options for the secondary node

* extract `init_parentchain` function in service and add `parentchain_id` to `set_nonce` and `set_metadata` ecalls (id is not used yet in the enclave)

* rename `get_node_metadata_repository_from_solo_or_parachain` to `get_node_metadata_repository_from_teerex_solo_or_parachain` and add `get_node_metadata_repository_from_secondary_solo_or_parachain`

* [enclave-runtime] `set_nonce` takes into account the parentchain id now

* fix test compilation

* add complete code for secondary parentchain

* [service/config] fix cli naming

* [service] indicate in logs that they are for the secondary parentchain

* [service] fix clippy

* [service] fix tests

* [enclave-runtime] fix: don't initialize second light client seal on the global for the first client

* [service/setup] fix purging second light client db.

* [service] print block number at which the enclave registered.

* [parentchain] remove some unnecessary trait bounds (simplify error message)

* [enclave-runtime] remove unused unit test

* OnchainOcallApi does not take a paramenter such that it nows, which parentchain to talk to.

* [stf-executor] remove code that executes a state update upon before stf execution as it is apparently no longer needed

* [service] propagate parentchain id throughout the codebase

* [service] fix tests

* [local-setup] prepare configs to run multiple nodes

* [stf-executor/executor] fix wrong import

* [local-setup] add a config that spawns two nodes

* [service] remove unnecessary traitbounds

* [service] minor logging fixes

* `sync_parentchain` and `trigger_parentchain_block_import` have the `ParentchainId` are too now.

* [service/worker_onchain_ocall] log to which parentchain we want to send the extrinsic

* [service/ocall_bridge] fix log typo

* [enclave-runtime] supress clippy warning

* [service] fix: fund enclave on secondary parentchain

* [service/worker_on_chain_ocall] improve log for extrinsic

* [local-setup] sleep 18 between setting up nodes, so that the logs are easier to distinguish

* add new indirect calls filer, which checks for transfers to alice to shield these funds

* [indirect-executor] use correct type in `TransferToAliceShieldsFundsFilter`

* [indirect-executor] better logging

* [local-setup/tmux_logger_two_nodes] only use three panes as the last one was unused

* [indirect-executor] remove unused imports

* [indirect-executor/transfer_to_alice_shields_funds] fix alice account

* [local-setup] use trace in indirect executor

* [cli] add `demo_shielding_unshielding_two_nodes` (working)

* rename `ParentchainId` enum variants to `Integritee` and `TargetA`

* [node-api/metadata] format pallet_balances licence header.

* [indirect-calls-executor] fix documentation

* [indirect-calls-executor] rename `ALICE_ENCODED` to `ALICE_SEED`

* [Enclave.edl] fix indents

* rename `get_node_metadata_from_teerex_solo_or_parachain` to `get_node_metadata_repository_from_integritee_solo_or_parachain`

* [enclave-runtime] rename error `NoTeerexParentchainAssigned` to `NoIntegriteeParentchainAssigned`

* [enclave-runtime] rename error `NoSecondaryParentchainAssigned` to `NoTargetAParentchainAssigned`

* [itp-nonce-cache] rename caches to `INTEGRITEE_NONCE_CACHE` and `TARGET_A_PARENTCHAIN_NONCE_CACHE`

* move nonce caches to the enclave-runtimes global components

* [enclave-runtime] change doc comments to regular comments were suitable

* [itp-settings] rename light-client db paths

* [enclave-runtime/initialization] rename parachain.rs to integritee_parachain.rs and `FullParachainHandler` to `IntegriteeParachainHandler`

* [enclave-runtime/initialization] rename parachain2.rs to target_a_parachain.rs and `FullParachainHandler2` to `TargetAParachainHandler`

* [enclave-runtime/initialization] rename solochain.rs to integritee_solochain.rs and `FullSolochainHandler` to `IntegriteeSolochainHandler`

* [enclave-runtime/initialization] rename solochain2.rs to target_a_solochain.rs and `FullSolochainHandler2` to `TargetASolochainHandler`

* [enclave-runtime/initialization] reflect previous changes in global components naming

* [enclave-runtime/initialization] consistent naming in global components

* [enclave-runtime/initialization] rename Teerex... types to Integritee... types

* [enclave-runtime/initialization] rename Secondary... types to TargetA... types

* [enclave-runtime/initialization] better distinguish the two light-client seals

* [enclave-runtime] some more minor renamings for new scheme

* [itp-settings] fix naming of target_a light client db path

* [enclave-runtime/global_components] fix grammar in doc

* [enclave-runtime/global_components] minor doc fixes

* [enclave-runtime/initialization] fix naming of light-client-db variables

* [cli] rename cli args in demo scripts to be more readable and use new names for parentchains

* fmt

* [enclave-runtime] more ergonomic code due to shorter variable names.

* [indirect-calls-executor] simplify creation of alice's account id

* [indirect-calls-executor] add an assert_eq in the doc test

* [service] update cli arguments to new parentchain naming

* [service/config] fix missing renamings

* [service/node_api_factory] add endpoint renamings

* [service/main] rename node_api to integritee_rpc_api

* [service/main] rename `init_parentchain` and `init_secondary_parentchain` to `init_integritee_parentchain` and `init_target_a_parentchain`

* [service/config] fix naming

* [service/main] fix moving of variables

* [service] fix tests

* [service/bridge-api] rename error to new chain naming

* [service/ocall_bridge] rename api factories for new naming

* consisten naming change remaning: target_a_chain -> target_a_parentchain

* fix missing minor renamings

* clippy

* [enclave-runtime/global-components] fix misallocation of `IntegriteeBlockImporter` to a Target A parentchain type. (this did not have any effect in the offchain-worker case)
@brenzi brenzi mentioned this pull request Nov 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brenzi brenzi brenzi approved these changes

Assignees
No one assigned
Labels
A0-core Affects a core part A4-offchain A5-teeracle B1-releasenotes C1-low 📌 Does not elevate a release containing this beyond "low priority" E3-hardmerge PR introduces a lot changes in a lot of files. Requires some effort to merge or rebase to.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@clangenb @brenzi