Skip to content
This repository has been archived by the owner on Oct 4, 2019. It is now read-only.

[WIP] External chain configuration, chain-specific data directories, configure chains from command line #189

Merged
merged 143 commits into from
May 2, 2017

Conversation

whilei
Copy link
Contributor

@whilei whilei commented Apr 19, 2017

PR/branch addresses the following issues:

Still TODO in the code:

  • Write new unit and integration tests to cover new code
  • Check coverage
  • Update CI tests; Travis, AppVeyor, bats
  • Tackle some TODOs and FIXMEs left in the comments, ie refactoring/logic-proofing opportunities
  • Standardize/prettify logging in some places

TODO otherwise:

  • Documentation:
  • Wiki
  • Readme
  • Proofread/consensus default names and other hardcoded user-facing values, ie
  • "EthereumClassic" vs "Ethereum-Classic" datadirs
  • "mainnet" vs "mainnet-classic"
  • JSON marshal tags (camelCase, hy-phenated, snake_case?)

There are still some open questions about parts of these changes, including:

  • Is it OK?/how-to-handle moving where we store users' data? Assuming we can make an automatic and seamless transition between directories within Geth, there is still the liability that we might break external scripts/libraries that depend on the original location.

[base data dir] /home/Ethereum ➡️ /home/EthereumClassic
[default chain data] /home/EthereumClassic/chaindata,nodes,keystore... ➡️ /home/EthereumClassic/mainnet/chaindata,nodes,keystore...

Possible solutions:

  1. Move it automatically, but provide a disclaimer/warning along with the new release.
  2. Move it optionally, either with a --migrate flag or interactive prompt or something.
  3. Don't move existing data, but new data created would follow new schemes.

  • Should the external config file settings be "hard" or "soft"? That is to say, if the config file specifies some but not all of the possible configuration fields (genesis, chainconfig, name, etc.), should geth panic, or quietly-but-with-logging fallback to use the default configs in those cases?

Currently implemented is "soft", without any flags. See this line of code for an example. changed

Possible solutions:

  1. Use "hard", assuming that in most cases JSON configuration files will have been created with dumpExternalChainConfig command and thus be complete (with good reason to error if they're not), as well as that they're wanting to rely completely on total customization.
  2. Use "soft", assuming that users will most generally want to override some but not all of the default configurations; it should just work.
  3. It doesn't matter, just provide an additional flag (--soft or --hard) to allow the user to override the default hard/soft setting.

(must have been overwritten in rebase selection)
(couldnt get rebase or merge to handle it nicely)
establish possible design for config override for impacted fields
still calling readJSONFile 3 times, which I'm not sure if is OK or could be improved wrt to memory
@@ -283,7 +283,7 @@ func CalcDifficulty(config *ChainConfig, time, parentTime uint64, parentNumber,
case "ecip1010":
if length, ok := f.GetBigInt("length"); ok {
explosionBlock := big.NewInt(0).Add(fork.Block, length)
if explosionBlock.Cmp(num) < 0 {
if num.Cmp(explosionBlock) < 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Real glad you caught this. See 012ba0f for test to ensure.

whilei added 8 commits May 1, 2017 04:59
solution: refactor chain configuration for context/json
solution: set global var, akin to ctx
solution: rename ChainNameFlag -> ChainIDFlag, matching JSON config file key
solution: use  flag to fall back to hash if can't find tags for whatever reason

fixes #196
solution: use implementation from original genesis dump.

not usre if it actually makes any difference, but seems safer
solution: panic on conflict-of-interest flags
whilei added 3 commits May 2, 2017 08:36
…es, inconsistent with convention

solution: use prefixedHex addresses as GenAlloc
solution: test for prefix and nonprefixed -hex address in 'init' command
solution: update to prefixedHex addresses for mainnet configs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants