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 chain properties to chain-spec-builder #7368

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Moliholy
Copy link
Contributor

This PR adds support for chain properties to chain-spec-builder. Now properties can be specified as such:

$ chain-spec-builder create -r $RUNTIME_PATH --properties tokenSymbol=DUMMY,tokenDecimals=6,isEthereum=false

/// Chain properties in comma-separated KEY=VALUE format.
/// Example: `--properties tokenSymbol=UNIT,tokenDecimals=12,ss58Format=42,isEthereum=false`
#[arg(long)]
pub properties: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub properties: Option<String>,
pub properties: Vec<String>,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 1b8e84a.

Comment on lines 86 to 87
/// Chain properties in comma-separated KEY=VALUE format.
/// Example: `--properties tokenSymbol=UNIT,tokenDecimals=12,ss58Format=42,isEthereum=false`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Chain properties in comma-separated KEY=VALUE format.
/// Example: `--properties tokenSymbol=UNIT,tokenDecimals=12,ss58Format=42,isEthereum=false`
/// Chain properties in `KEY=VALUE` format.
///
/// Multiple `KEY=VALUE` entries can be specified and separated by a comma.
///
/// Example: `--properties tokenSymbol=UNIT,tokenDecimals=12,ss58Format=42,isEthereum=false`
/// Or: `--properties tokenSymbol=UNIT --properties tokenDecimals=12 --properties ss58Format=42 --properties=isEthereum=false`
///
/// The first uses comma as separation and the second passes the argument multiple times. Both
/// styles can also be mixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in 1b8e84a.

Comment on lines 397 to 398
let key = iter.next().unwrap_or("").trim().to_owned();
let value_str = iter.next().unwrap_or("").trim();
Copy link
Member

Choose a reason for hiding this comment

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

If they don't exist, it should be an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 1b8e84a.

@bkchr bkchr added the T0-node This PR/Issue is related to the topic “node”. label Jan 29, 2025
fn parse_properties(raw: &String, props: &mut sc_chain_spec::Properties) -> Result<(), String> {
for pair in raw.split(',') {
let mut iter = pair.splitn(2, '=');
let key = iter.next().ok_or("Invalid chain property key")?.trim().to_owned();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let key = iter.next().ok_or("Invalid chain property key")?.trim().to_owned();
let key = iter.next().ok_or_else(|| format!("Invalid chain property key: {pair}"))?.trim().to_owned();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in d9b1292.

@bkchr bkchr enabled auto-merge January 29, 2025 21:34
auto-merge was automatically disabled January 29, 2025 21:39

Head branch was pushed to by a user without write access

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants