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

Adds ability to specify chain type in chain-spec-builder #4542

Merged
merged 9 commits into from
May 31, 2024
Merged

Adds ability to specify chain type in chain-spec-builder #4542

merged 9 commits into from
May 31, 2024

Conversation

gupnik
Copy link
Contributor

@gupnik gupnik commented May 22, 2024

Currently, chain-spec-builder only creates a spec with Live chain type. This PR adds the ability to specify it while keeping the same default.

@gupnik gupnik added the T0-node This PR/Issue is related to the topic “node”. label May 27, 2024
@kianenigma
Copy link
Contributor

kianenigma commented May 29, 2024 •
edited

I wonder though, does this make any tangible difference other than changing the ChainType in the json file?

What would be cool if the sp_keyring accounts are all injected into the JSON file.

cc @michalkucharczyk should the runtime be aware of the chain_type and make an informed decision here?

@xlc
Copy link
Contributor

xlc commented May 29, 2024

I feel it is easier to just have another tool to modify the JSON file

@gupnik
Copy link
Contributor Author

gupnik commented May 29, 2024 •
edited

I wonder though, does this make any tangible difference other than changing the ChainType in the json file?

With this change, PJS is able to show development accounts for chain types other than live. They do not carry any balance though at the moment. Happy to look into including that as well. For now, I had modified get_preset to add some balance to these accounts.

Without this change, I was unable to interact with PJS using development accounts at all.

@kianenigma
Copy link
Contributor

Yeah I am also leaning toward the fact that if this is just modifying the JSON file, it can be done with hand.

@michalkucharczyk
Copy link
Contributor

What would be cool if the sp_keyring accounts are all injected into the JSON file.

Do we know how to serialize keys to json (which is domain of runtime)?

cc @michalkucharczyk should the runtime be aware of the chain_type and make an informed decision here?

Well I like to ask question does this property belong to runtime domain?
If this is a kind of flag that enables some debug features in app, I would not say. It is a property of network/nodes, so probably should stay outside runtime. You probably would not like to recompile runtime to enable it.

@michalkucharczyk
Copy link
Contributor

Adding flags like this is fine for me. No need to run additional tool.

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6349553

@gupnik gupnik added this pull request to the merge queue May 31, 2024
Merged via the queue into master with commit 78c24ec May 31, 2024
155 of 158 checks passed
@gupnik gupnik deleted the gupnik/chain-spec branch May 31, 2024 02:35
ordian added a commit that referenced this pull request Jun 4, 2024
@ordian
Merge branch 'master' into ao-fix-backing-filtering-on-disputes
e91b425 
* master: (106 commits)
  [ci] Delete unused flow (#4676)
  Fix umbrella CI check and fix the C&P message (#4670)
  Add Dockerfiles to the templates (#4637)
  Revamp the Readme of the minimal template (#4649)
  Add chain-spec-builder docker image (#4655)
  Update Amforc bootnodes for Kusama and Polkadot (#4668)
  make all storage items in parachain-system public (#4645)
  [Pools] Refactors and runtime apis for DelegateStake (#4537)
  update amforc westend and its parachain bootnodes (#4641)
  Better error for missing index in CRV2 (#4643)
  Implement `XcmPaymentApi` and `DryRunApi` on all system parachains (#4634)
  Use Unlicense for templates (#4628)
  collator-protocol: remove `elastic-scaling-experimental` feature (#4595)
  Update `runtime_type` ref doc with the new "Associated Type Bounds" (#4624)
  Adds ability to specify chain type in chain-spec-builder (#4542)
  Fix broken windows build (#4636)
  Beefy client generic on aduthority Id (#1816)
  pallet-staking: Put tests behind `cfg(debug_assertions)` (#4620)
  Broker new price adapter (#4521)
  Change `XcmDryRunApi::dry_run_extrinsic` to take a call instead (#4621)
  ...
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this pull request Jun 5, 2024
@kianenigma @hitchhooker
Adds ability to specify chain type in chain-spec-builder (paritytech#…
0277916 
…4542)

Currently, `chain-spec-builder` only creates a spec with `Live` chain
type. This PR adds the ability to specify it while keeping the same
default.

---------

Co-authored-by: Kian Paimani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalkucharczyk michalkucharczyk michalkucharczyk approved these changes

@bkchr bkchr bkchr left review comments

@kianenigma kianenigma kianenigma approved these changes

Assignees
No one assigned
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@gupnik @kianenigma @xlc @michalkucharczyk @paritytech-cicd-pr @bkchr