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

Add cli command to load genesis config of the evm domain in json format #1858

Merged
merged 5 commits into from
Aug 23, 2023

Conversation

NingLin-P
Copy link
Member

This PR adds a domain subcommand to load the genesis config of the evm domain in json format. This is mainly used to complement #1857 to generate the raw_genesis argument for the instantiate_domain extrinsic.

Usage :

subspace-node domain load-genesis-config --help
subspace-node domain load-genesis-config --chain gemini-3f > gemini-3f-evm-domain-genesis-configs.json

cc @vedhavyas @nazar-pc

Code contributor checklist:

Signed-off-by: linning <linningde25@gmail.com>
Copy link
Member

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

Hm... load is when it takes a chain spec and loads into something internal. This is the opposite of that.

Can we not have it in subspace-node? Having it in subspace-node means we can't get rid of all EVM dependencies when building it, while the ideal situation would be that subspace-node can run generic domains and knows nothing about EVM, especially doesn't pull a huge list of its dependencies.

Having it separately would mean it is possible to adjust to more domains rather than pulling every possible domain into subspace-node.

@vedhavyas
Copy link
Contributor

Can we not have it in subspace-node? Having it in subspace-node means we can't get rid of all EVM dependencies when building it, while the ideal situation would be that subspace-node can run generic domains and knows nothing about EVM, especially doesn't pull a huge list of its dependencies.

Do you prefer not having subspace-node as entry point for domain subcommands like benchmarking etc...? If so, we can have a new binary under domains.

Hm... load is when it takes a chain spec and loads into something internal. This is the opposite of that.

I think build-genesis-config would be more appropriate

@nazar-pc
Copy link
Member

Do you prefer not having subspace-node as entry point for domain subcommands like benchmarking etc...? If so, we can have a new binary under domains.

Yes, I think everything that results in runtime of any particular domain being pulled into subspace-node should be moved out. Separate binary under domains makes sense to me.

@NingLin-P
Copy link
Member Author

while the ideal situation would be that subspace-node can run generic domains and knows nothing about EVM, especially doesn't pull a huge list of its dependencies.

Sorry I don't get it, IIUC subspace-node is the outermost layer, any generic type argument needs to be specified here, if we want to get rid of all the domain-related dependencies that means we need to remove all the domain stuff from subspace-node and deprecate how we start the operator node now:

subspace-node [consensus-chain-args] -- [domain-args]

@nazar-pc
Copy link
Member

and deprecate how we start the operator node now

As long as domain has expected runtime API node should still be able to run it, but from subspace-node's point of view it should be "some generic domain", it shouldn't need to know what that domain is, just that it conforms to expected interfaces.

@NingLin-P
Copy link
Member Author

But subspace-node should be able to run any domain that is supported, and to run a domain node we will have to construct its native runtime which requires all its dependencies.

Unless we can get rid of the native runtime and use the WASM runtime only otherwise the dependencies of all the supported domains will still need to build into the subspace-node binary in order to support running these domain with subspace-node.

@nazar-pc
Copy link
Member

But subspace-node should be able to run any domain that is supported, and to run a domain node we will have to construct its native runtime which requires all its dependencies.

Unless we can get rid of the native runtime and use the WASM runtime only otherwise the dependencies of all the supported domains will still need to build into the subspace-node binary in order to support running these domain with subspace-node.

I do not think we need native runtime, moreover, it is going away in the future entirely. So we should be able to use some dummy runtime just to satisfy Substrate API in the meantime and force WASM runtime all the time for domains.

It is clearly suboptimal and non-sustainable to have all runtimes in subspace-node.

@NingLin-P
Copy link
Member Author

I do not think we need native runtime, moreover, it is going away in the future entirely. So we should be able to use some dummy runtime just to satisfy Substrate API in the meantime and force WASM runtime all the time for domain

It is clearly suboptimal and non-sustainable to have all runtimes in subspace-node.

Okay, I get your point now, but it seems non-trivial work for both 1. moving all domain subcommands to a separated binary domains and 2. adding a dummy runtime to get rid of the domain native runtime. Would prefer the proceed with this PR as it is (with the naming change) and I will create tracking issues for the above 2 works.

@nazar-pc
Copy link
Member

Certainly proceed as is, I'm just explaining where we want to go.

Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P
Copy link
Member Author

The command is renamed to build-genesis-config now, the last commit removes a needless dependency for subspace-runtime which is a leftover of #1847. PTAL

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