Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Dev chain #3385

Merged
merged 13 commits into from
Nov 14, 2016
Merged

Dev chain #3385

merged 13 commits into from
Nov 14, 2016

Conversation

keorn
Copy link

@keorn keorn commented Nov 11, 2016

parity --chain dev runs InstantSeal Engine. The chain has account created with "" phrase prepopulated with large balance.

Useful for UI and Dapp development.

@keorn keorn added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Nov 11, 2016
pub fn new_ethereum_olympic() -> Spec { load_bundled!("ethereum/olympic") }

/// Create a new Frontier mainnet chain spec.
pub fn new_ethereum_frontier() -> Spec { load_bundled!("ethereum/frontier") }
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just introduce a new module Ethereum rather than inserting _ethereum into the names manually.

Copy link
Author

Choose a reason for hiding this comment

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

I thought the best place would be Spec constructor methods instead of free functions, but I don't think these can be namespaced on the call site, thus the addition. Should they be made functions?

Copy link
Contributor

Choose a reason for hiding this comment

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

free functions (as they were) are fairly idiomatic, especially when there are as many different presets as we have.

@gavofyork gavofyork added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 12, 2016
@keorn keorn added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Nov 12, 2016
@keorn keorn added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Nov 14, 2016
@@ -135,6 +135,12 @@ impl From<ethjson::spec::Spec> for Spec {
}
}

macro_rules! load_bundled {
($e:expr) => {
Spec::load(include_bytes!(concat!("../../res/", $e, ".json")) as &[u8]).expect("Chain spec is invalid")
Copy link
Contributor

Choose a reason for hiding this comment

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

the error messages here might get a little worse than before. the concat! macro can customize the expect message as well.

Copy link
Author

Choose a reason for hiding this comment

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

This is purely for us if we decide to add an invalid spec, it will not even compile then.

Copy link
Contributor

Choose a reason for hiding this comment

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

seems to me that the error wouldn't get triggered until runtime

Copy link
Author

Choose a reason for hiding this comment

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

ah yes, I confused with include_bytes. Improved now.

@rphmeier rphmeier added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 14, 2016
@rphmeier
Copy link
Contributor

LGTM other than tiny grumble

@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Nov 14, 2016
@gavofyork gavofyork merged commit ae67bd5 into master Nov 14, 2016
@gavofyork gavofyork deleted the dev-chain branch November 14, 2016 15:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants