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

Make NetworkConfig serializable #873

Closed
onbjerg opened this issue Jan 13, 2023 · 1 comment · Fixed by #897
Closed

Make NetworkConfig serializable #873

onbjerg opened this issue Jan 13, 2023 · 1 comment · Fixed by #897
Assignees
Labels
A-networking Related to networking in general C-enhancement New feature or request

Comments

@onbjerg
Copy link
Member

onbjerg commented Jan 13, 2023

Describe the feature

The idea of the Config struct used in the CLI is that it is a thin wrapper around existing config structs to the extent possible to lower maintenance, i.e. instead of having to add new config in 2 places, you just add it in one.

Currently though, it is not possible to serialize or deserialize the NetworkConfig struct (or NetworkConfigBuilder) because:

  1. It takes a generic parameter C as a stand-in for the client.
  2. It holds a Box<dyn BlockImport>

There are a few possible paths here:

  1. We try to make NetworkConfigBuilder serializable by moving the non-serializable parts into the build function, or
  2. We make NetworkConfig serializable, which means the networking components need to take the unserializable things separately from the config,
  3. We make a new slimmer version of NetworkConfig that is Serialize and Deserialize, and we have NetworkConfig hold one of these

Additional context

As part of this issue we would also need to deduplicate a few things. Currently we have a PeersConfig as part of the Config struct, but this is also present on NetworkConfig.

@onbjerg onbjerg added C-enhancement New feature or request A-networking Related to networking in general labels Jan 13, 2023
@leruaa
Copy link
Contributor

leruaa commented Jan 16, 2023

I can take this on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-networking Related to networking in general C-enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants