Papyrus config refactor about papyrus

[MartianGreed]

I' down to work on this one !

[AbdelStark]

I' down to work on this one !

let's go!

[MartianGreed]

@EvyatarO @abdelhamidbakhta just to be clear guys, you want to have a single struct Config (as there already is) that can be partially configured from yaml / env and with cli arguments that are overriding configuration if they are provided ?

Basically we could imagine an api working as following:

let providers = vec![EnvConfigProvider::new(), YamlFileConfigProvider::new(), CliArgumentsConfigProvider::new( args().collect())];
let config = Config::load(providers)?;

Given you want to keep configuration in a single place, we won't be able to use automatic deserialization.
Is the ConfigProvider trait a good track or you want something like macros ?

[dan-starkware]

Not sure I understand what you meant by not being able to use automatic deserialization.
A ConfigProvider trait seems a better approach than macros IMO.

To make things clearer:

• We want users to be able to run a node without supplying all of the config, so we need default config.
• We want to allow overriding the config by a configuration file (file), environment variables (env) and command line arguments (CLA), in that precedence (default -> file -> env -> CLA)

Currently

• We don't support env at all.
• The code is not maintainable. In order to change/introduce a new configuration we need to edit several places. Default values / descriptions may not be consistent throughout the code.

[MartianGreed]

Thank's for your answer Dan !

Not sure I understand what you meant by not being able to use automatic deserialization.

I mean automatic deserialization into a struct

I'm also conviced than trait will be better than macros :)

Ok, this is clear, I will propose an implementation based on traits !

[MartianGreed]

Hey @dan-starkware I'm currently implementing the ConfigProvider trait. I'm almost done with yaml.

Does any key/value pairs present in Yaml can be also overriden by env or cla ?

For the moment I'll implement it as empty (just return previous config) lmk if you want me to add those default values :)

[yair-starkware]

@MartianGreed, thank you for your contribution.
Some configurations in the Yaml can be overridden by cla, for example, chain_id and storage path.
At the moment, we don't have support for configuration from env.

[MartianGreed]

Hi @yair-starkware, I did some basic implementation for these configurations. PR is open lmk how things can be improved ;)

[MartianGreed]

Hey @yair-starkware sorry for the delay I've been very busy lately, I will work on this during this weekend... Sorry for the late response and the absence of informations..

[dan-starkware]

Thanks for the update. If you want we can schedule a short zoom to make sure we are in sync with the proposed approach.

[MartianGreed]

Hi @dan-starkware sure, let me know when we can make it !
I've went over the solution you proposed and I guess I understood so we can have a call to be sure everything is well understood.
One question, why not use a config.toml file directly ? This way we can manage to serialize it easily, if not I can manage to implement it manually

[psnewer]

Hi, @dan-starkware @yair-starkware I have went through this issue and would like to go on with it. Here are my thoughts:

1. Define a config metadata file from which everything else are derived from. This config metadata file should contain the default values, long names, short names, environment names, and descriptions for the parameters. Specifically, the custom config file should also be one of the parameters.
2. I do not want to maitain the structure of metadata config in code file overmuch when add or delete configuration. The content of the config metadata file will be read into a hashmap directly. Similarly, values of parameters in the config meatadata file will be loaded into another hashmap. The priority order for parameter value will be as follows: command line > environment > custom config file > default value.
3. The Configurable trait with new() and dump() functions for every component config will be implemented using derive macro. Thus we do not need to carry out the Configurable trait for each component config separately. Additionally, the parameters defined in the config metadata file are considered to be at the smallest granularity. Each parameter in component config can be a combination of them, and the new() and dump() functions will use a recursive approach to find the corresponding value for each component config parameter. For example, if the config metadata file does not define the gateway parameter of type GatewayConfig, but it defines parameters like server_address under the GatewayConfig type, we can still find the value of gateway based on the struct definition of GatewayConfig. Furthermore, due to the use of derive macro, we only need to declare the parameter in the component config. If the parameter or its members are already defined in the config metadata file, we can obtain the value of it.