ChainSpecEmbeddedResource

[smartprogrammer93]

Resolves #4266

Changes:

Chainspec to be in embedded resources and read from there without requiring users to make any changes after updating. The only users who will have to make changes after an update are the ones running private networks.

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation Update
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Other (please describe):

Testing

Requires testing

• Yes
• No

In case you checked yes, did you write tests??

• Yes
• No

[LukaszRozmej]

Can we make it smart somehow?

Ideas to brainstorm:

1. If it isn't an obvious path (directories or file extensions) then read from embedded resource.
   ../mainnet.cfg - file
   mainnet.cfg - file (or maybe force resource?)
   mainnet - resource

2. If read from file but we have same resource then compare file and resource and if we found differences then put a warning that the chainspec may not be up to date.

3. Change all built in configs to go to resources.

[rubo]

After a discussion with @smartprogrammer93, I see the solution as follows.

To stay backward-compatible as much as possible, we keep the ChainSpecPath parameter. If its value is the current default, i.e. chainspec/<network>.json (or simply <network>), then the embedded one is used. Otherwise, it tries to load an external file using the value of ChainSpecPath as a path.

This should work for most of the users without breaking anything:

• Users using the default chainspec and default path will always have the latest chainspec from us.
• Users using a custom chainspec with the default path will have their chainspec overridden.
• Users using a custom chainspec with a custom path should handle any change themselves. It's fair.
• Users using the default chainspec with a custom path should start using the default path. It doesn't cost them anything as the files are now included in the binary, so they can safely set ChainSpecPath to just <network>.

I don't think we need to make the process any smarter for now. Later, based on user feedback, we can consider a better solution.

[LukaszRozmej]

After a discussion with @smartprogrammer93, I see the solution as follows.

To stay backward-compatible as much as possible, we keep the ChainSpecPath parameter. If its value is the current default, i.e. chainspec/<network>.json (or simply <network>), then the embedded one is used. Otherwise, it tries to load an external file using the value of ChainSpecPath as a path.

This should work for most of the users without breaking anything:

• Users using the default chainspec and default path will always have the latest chainspec from us.
• Users using a custom chainspec with the default path will have their chainspec overridden.
• Users using a custom chainspec with a custom path should handle any change themselves. It's fair.
• Users using the default chainspec with a custom path should start using the default path. It doesn't cost them anything as the files are now included in the binary, so they can safely set ChainSpecPath to just <network>.

I don't think we need to make the process any smarter for now. Later, based on user feedback, we can consider a better solution.

I'm a bit hesitant that we will override the default paths to built in chainspec. I don't like to be smarter than user. But not saying it's good or bad idea. Would like to have more input from others.

[Demuirgos]

Is the field "ReadChainspecFromFile" necessary in the "InitConfig" class?
I think we can determine which mode we're using based on the ChainspecFilePath value, if it is a correct path (I think a fully qualified path or a relative path) then I think it would make sense to use IChainspecLoader.LoadFromFile, otherwise if the pth input is "chainspec/{specname}.json" or "{specname}.json" or "{specname}" the we load it from embedded resouces.

[smartprogrammer93]

We are removing the flag. The latest suggestion is to look for the file in embedded resource, if it is not there, we will read the file.

[Demuirgos]

2. If read from file but we have same resource then compare file and resource and if we found differences then put a warning that the chainspec may not be up to date.

Will this method include this suggestion?

[smartprogrammer93]

We can include a warning that we are not using the chainspec from file if the file exists, but i dont think we should mention it being outdated or not.

[Demuirgos]

Wouldn't it be better if the user got notified that the client isn't using the chainspec file they provided with the reason why as a feedback?
I think whether we load their file or our embedded file in the case of collisions is something that needs clarification as well, and I believe the warning is a good thing to have.

[rubo]

I'd also suggest always using file scope namespaces. It eliminates redundant nesting. Simply put ; after the namespace and Visual Studio will do the rest.

[LukaszRozmej]

Some smaller comments about readability and code organisation

[rubo]

@MarekM25 I tested this per your request as follows:

• When no chain spec file is specified
• When a custom chain spec file is specified
• When a custom chain spec file has the same path as the default

It works as expected.

[kamilchodola]

Checked:

1. No flag used - using embedded resource (OK)
2. Flag specified to existing file but which name is exactly the same as embedded one - using embedded resource (OK)
3. Flag specified to new file - using file (OK)
4. Flag specified to real chainspec but in location different than chainspec directory - using file (seems ok - we should be backward compatible)
5. Setting flag to chainspec/goerli.json having this json empty (remove all content) - using embedded (ok)