implement per-parentchain extrinsic factory and metadata by brenzi · Pull Request #1501 · integritee-network/worker

brenzi · 2023-11-20T16:37:32Z

needed to support Asset Hub (or Encointer) as Target A parentchain

major steps:

per-parentchain Extrinsic Type (especially AssetTip)
per-parentchian metadata (pallet indices)

…c functions

…at I already did

…flicts

brenzi · 2023-11-22T19:38:07Z

service/src/account_funding.rs

+where
+	ParentchainApi: AccountApi<AccountId = AccountId, Balance = Balance, Index = Index>
+		+ GetBalance<Balance = Balance>
+		+ GetTransactionPayment<Balance = Balance>
+		+ BalancesExtrinsics<
+			Address = Address,
+			Balance = Balance,
+			Extrinsic<TransferAllowDeathCall<Address, Balance>> = UncheckedExtrinsicV4<
+				Address,
+				TransferAllowDeathCall<Address, Balance>,
+				<ParentchainApi as ChainApi>::Signature,
+				<ParentchainApi as ChainApi>::SignedExtra,
+			>,
+		> + SubmitAndWatch<Hash = Hash>
+		+ ChainApi<Signer = sr25519::Pair>,
+	ParentchainApi::Signature: Encode,
+	ParentchainApi::SignedExtra: Encode,
+{


this is so horribly verbose. there has to be another way

You can create an aggregate trait like here: https://stackoverflow.com/a/26983395/19331449

brenzi · 2023-11-22T19:40:03Z

service/src/main_impl.rs

+	(parentchain_handler, last_synced_header)
+}
+
+fn init_integritee_parentchain<E>(


this was just an attempt to solve the mess with redundant but explict code. not sure that's the way

brenzi · 2023-11-22T19:40:42Z

service/src/main_impl.rs

 }

-fn send_extrinsic(
+fn send_integritee_extrinsic(


no need to be generic over parentchains. we only need this for Integritee Network

brenzi · 2023-11-22T19:42:30Z

service/src/parentchain_config.rs

+pub type TargetBRuntimeConfig =
+	WithExtrinsicParams<AssetRuntimeConfig, target_b::ParentchainExtrinsicParams>;
+
+pub enum ParentchainApiLocal {


this I somehow like and it seems to work for OCall. Maybe we could make it work for service-main instead of the ParentchainApiWrapper shizzle?

Yes, I think this is the way to go actually for many things, also when we distinguish build flavors.

brenzi · 2023-11-22T19:43:19Z

service/src/parentchain_config.rs

+// fixme: this is an ugly hack because api-client's 'new()' isn't part of any trait we could implement therefore we can't use the fn new() on generic api types
+impl ParentchainApiWrapper for IntegriteeParentchainApiWrapper {


really ugly. there has to be a better way. but we may need to patch api-client

Wrapper not necessary, we can just define our own trait and implement it on the api client IMO, which is a bit better.

brenzi · 2023-11-22T19:44:17Z

enclave-runtime/src/initialization/parentchain/target_a_parachain.rs

 				extrinsics_factory.clone(),
 			)?,
 			WorkerMode::Sidechain =>
-				unimplemented!("Can't run target a chain in sidechain mode yet."),


Is there anything I fail to see why this will not work?

brenzi · 2023-11-22T19:53:19Z

@clangenb I think I need an intermediate review as I seem to get something entirely wrong.

status:

a-priori: master starts against Zombienet with Assethub. Everything on Integritee works fine and sidechain blocks are produced, shard vault created a.s.o., but extrinsic to Asset Hub to fund the enclave fails, as has to be expected. So that's a healthy starting point IMO
this WIP still has introduced a lot of redundant code, a cleanup will be necessary once it works.
enclave-runtime builds in SW,sidechain,dcap mode with per-parentchain extrinsic factories and ocall refactoring seems fine

service main is a PITA. I'm particularly concerned about one of your remarks here:

worker/service/src/parentchain_handler.rs

Lines 66 to 67 in 922e873

    
           // #TODO: #1451: Reintroduce `ParentchainApi: ChainApi` once there is no trait bound conflict 
        
           // any more with the api-clients own trait definitions.

reproduce my errors in root folder build: SGX_MODE=SW WORKER_MODE=sidechain WORKER_FEATURES=dcap make

clangenb · 2023-11-22T23:24:01Z

app-libs/parentchain-interface/Cargo.toml

 # substrate dep
 binary-merkle-tree = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.42" }
 frame-support = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.42" }
+frame-system = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.42" }


std enablement

clangenb · 2023-11-22T23:24:43Z

core-primitives/extrinsics-factory/src/lib.rs

 #[cfg(all(feature = "std", feature = "sgx"))]
 compile_error!("feature \"std\" and feature \"sgx\" cannot be enabled at the same time");

+extern crate core;


clangenb · 2023-11-22T23:26:44Z

service/src/parentchain_config.rs

+// fixme: this is an ugly hack because api-client's 'new()' isn't part of any trait we could implement therefore we can't use the fn new() on generic api types
+impl ParentchainApiWrapper for IntegriteeParentchainApiWrapper {


Wrapper not necessary, we can just define our own trait and implement it on the api client IMO, which is a bit better.

clangenb · 2023-11-22T23:28:22Z

service/src/parentchain_config.rs

+pub type TargetBRuntimeConfig =
+	WithExtrinsicParams<AssetRuntimeConfig, target_b::ParentchainExtrinsicParams>;
+
+pub enum ParentchainApiLocal {


Yes, I think this is the way to go actually for many things, also when we distinguish build flavors.

clangenb · 2023-11-22T23:37:50Z

service/src/parentchain_handler.rs

 // #TODO: #1451: Reintroduce `ParentchainApi: ChainApi` once there is no trait bound conflict
 // any more with the api-clients own trait definitions.
-impl<EnclaveApi> ParentchainHandler<ParentchainApi, EnclaveApi>
+impl<EnclaveApi> ParentchainHandler<IntegriteeParentchainApi, EnclaveApi>


You are exactly right. I believe this is the root cause of the problem. The api-client has re-defined types and traits because they were not available in no-std in substrate < v0.9.43. When I updated the api-client to its newest version, but v0.9.42, I had to reintroduce these types again: scs/substrate-api-client@ea49a0d.

Then I was essentially in the same rabbit whole as you: certain trait-bounds weren't satisfied because we are using the actual types from substrate whereas the api-client uses its types/traits in its traitbounds.

My recommedation is: if you have to use generics in those api taits, we unfortunately have to upgrade the polkadot version and the api-client.

brenzi · 2023-11-23T10:15:30Z

will pause work here, hoping for teaclave to bump rustc so we can upgrade to Polkadot 1.3.0 and api-client 0.15.0
trait bound trouble should be easier to fix nicely then

implemented per-parentchain extrinsic factory

c2b8264

brenzi changed the title ~~implemented per-parentchain extrinsic factory~~ implement per-parentchain extrinsic factory and metadata Nov 20, 2023

brenzi added 11 commits November 21, 2023 09:03

putting the pieces together. enclave builds

a3b2bb8

before introducing a new Api trait for set_signer and many other basi…

9df2a35

…c functions

extended ChainApi with set_signer()

a67aa5d

fixed stuff but seing very strange errors suggesting to do exactly wh…

f11f36a

…at I already did

before trying to use enum with Api's

31d93b3

such a trait and associated type mess

230dd8d

a bit more consistent

4e2dbf5

seemingly fixed extrinsic type nailing for account_funding. really ugly

2e39440

not sure if we want this: separate init fn for integritee

a28259e

revert changes to parentchain_handler due to earlier remark about con…

c093289

…flicts

WIP. giving up for now

5d82bc8

brenzi commented Nov 22, 2023

View reviewed changes

clangenb reviewed Nov 22, 2023

View reviewed changes

minor review fixes

a675dcf

		// fixme: this is an ugly hack because api-client's 'new()' isn't part of any trait we could implement therefore we can't use the fn new() on generic api types
		impl ParentchainApiWrapper for IntegriteeParentchainApiWrapper {

Conversation

brenzi commented Nov 20, 2023 • 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!

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!

Choose a reason for hiding this comment

Uh oh!

brenzi commented Nov 22, 2023 • 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!

Choose a reason for hiding this comment

Uh oh!

clangenb Nov 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brenzi commented Nov 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

brenzi commented Nov 20, 2023 •

edited

Loading

brenzi commented Nov 22, 2023 •

edited

Loading

clangenb Nov 22, 2023 •

edited

Loading