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

port encointer client for parachain #3

Closed
brenzi opened this issue Jan 14, 2021 · 10 comments
Closed

port encointer client for parachain #3

brenzi opened this issue Jan 14, 2021 · 10 comments
Assignees

Comments

@brenzi
Copy link
Member

brenzi commented Jan 14, 2021

Because the parachain template still doesn't use MultiAddress, we need to adapt substrate-api-client as well as encointer-client to work with the encointer-collator

The strategy:

  • have the client in its own repo encointer/encointer-client
  • refactor encointer-client crate to be a library offering all chain-specific functions
  • separate encointer-cli-client crate for a CLI binary
  • make cli client support all chains with a single binary: Gesell, Cantillon, Parachain. Identify chain by metadata and use types accordingly (at least the Address type)
    • this will probably require making substrate-api-client more generic
@clangenb clangenb self-assigned this Jan 20, 2021
@clangenb
Copy link
Member

clangenb commented Jan 20, 2021

Summary of some research:

  • Address Type: The address type itself is not exposed in the metadata. This needs to be configured manually as explained in the polkadot-js FAQ.

  • Be More Generic Currently, it is only the Address type that differs, but for flexibility I think we should make the substrate-api-client generic over a Runtime trait as done in subxt, which defines all the primitives. So third-party developers can implement the trait themselves for a custom runtime. I would go for this approach. The substrate-api-client would look like this: ApiClient<T: Runtime>.

  • Runtime Metadata: This could be problematic too because the substrate-api-client infers its metadata based on the RuntimeMetadataPrefixed defined in substrate, which always deprecates earlier versions. However, this is only a potential problem in the future, as our nodes and the parachain-template currently use the same metadata version.

Implication for encointer-client

  • The cli client can easily support all the chains with different implementations of this newly introduced Runtime trait for each chain respectively. We instantiate the ApiClient with the appropriate Runtime depending on the metadata.
  • We hope that we maintain the same metadata version for all our chains and address that problem if the time comes.

@brenzi
Copy link
Member Author

brenzi commented Jan 20, 2021

chain specific trait does not help because we're still hard-coding the runtime into the binary. My goal would've been to share the same binary for different chains.
Probably, Gesell and Cantillon might merge into one anyway: Offering all pallets onchain too, so communities can decide if they want to deploy onchain or on TEE (immature thought).

What we could do:

  1. use MultiAddress for parachain -> so we have compatibility
  2. use Gesell client, but add substratee-pallet to Gesell
  3. add subcommand trusted for all the TEE stuff

thoughts?

@clangenb
Copy link
Member

clangenb commented Jan 20, 2021

  1. Should be done anyhow, I think. Roccoco does use the MultiAddress too.

I need to think about the 2. and 3.

@clangenb
Copy link
Member

clangenb commented Jan 20, 2021

Regarding chain specific trait. I did understand you correctly. We can pass the concrete Runtime trait implementation at runtime and not at compile time. This, however, implies that the Runtime trait needs to be object safe.

But I mentioned in this case the wrong signature above. It would be: ApiClient<dyn Runtime>

@clangenb
Copy link
Member

Probably, Gesell and Cantillon might merge into one anyway: Offering all pallets onchain too, so communities can decide if they want to deploy onchain or on TEE (immature thought).

If you think this is going happen, we should add the subcommand trusted, as there is no way for the binary to know if it should call the TEE or the node.

@clangenb
Copy link
Member

clangenb commented Jan 20, 2021

Conclusively, my suggestion would be:

  1. Update parachain to use MultiAddress. It deprecates the old Address type.
  2. Make the api-client ApiClient<dyn Runtime>. it does not take long to figure out if this is possible. If we can constrain the types we want to add to our Runtime trait with : Sized, it should work.
  3. Add trusted subcommand according to my reasoning above.

@clangenb
Copy link
Member

clangenb commented Jan 20, 2021

use Gesell client, but add substratee-pallet to Gesell

Why does it matter whether Gesell contains the substratee-pallet?

@clangenb
Copy link
Member

Make the api-client ApiClient. it does not take long to figure out if this is possible. If we can constrain the types we want to add to our Runtime trait with : Sized, it should work.

I just realized, there is no need for this if we update the parachain...

@brenzi
Copy link
Member Author

brenzi commented Feb 5, 2021

as the parachain now works with encointer-client-notee, shall we close this issue and postpone moving it to its own crate?

@clangenb
Copy link
Member

clangenb commented Feb 5, 2021

I agree to closing and postponing

@brenzi brenzi closed this as completed Feb 6, 2021
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

No branches or pull requests

2 participants